fix(ui): BundleAssets - add "No chunk" filter #4850
Conversation
WalkthroughThe changes introduce a new constant Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
#11694 Bundle Size — 380.81KiB (+0.03%).0cb8759(current) vs 9162b8a master#11692(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
| Current #11694 |
Baseline #11692 |
|
|---|---|---|
 Initial JS |
333.93KiB(+0.03%) |
333.81KiB |
 Initial CSS |
46.89KiB |
46.89KiB |
 Cache Invalidation |
28.53% |
0% |
| Chunks |
3 |
3 |
| Assets |
4 |
4 |
| Modules |
699 |
699 |
| Duplicate Modules |
0 |
0 |
| Duplicate Code |
0% |
0% |
| Packages |
39 |
39 |
| Duplicate Packages |
1 |
1 |
Bundle size by type 1 change 1 regression
| Current #11694 |
Baseline #11692 |
|
|---|---|---|
| JS |
333.93KiB (+0.03%) |
333.81KiB |
| CSS |
46.89KiB |
46.89KiB |
Bundle analysis report Branch fix-asset-chunk-filter Project dashboard
Generated by RelativeCI Documentation Report issue
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/ui/src/components/bundle-assets/bundle-assets.utils.ts (1)
171-181: Consider adding a comment to explain the "No chunk" filter placement.The implementation is correct and follows the existing patterns. The placement at the start of the children array makes sense as it's a special case.
Consider adding a comment to explain why the "No chunk" filter is placed first:
const chunksFilter: FilterGroupFieldData = { label: 'Chunks', children: [ + // Place "No chunk" filter first as it's a special case { key: NO_CHUNK_FILTER, label: 'No chunk', defaultValue: filters[`${ASSET_CHUNK}.${NO_CHUNK_FILTER}`] ?? true, }, ], };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/ui/src/components/bundle-assets/bundle-assets.utils.ts(4 hunks)
🔇 Additional comments (2)
packages/ui/src/components/bundle-assets/bundle-assets.utils.ts (2)
22-22: LGTM: Constant declaration is clear and follows conventions.
The constant name and value are descriptive and appropriate for its purpose.
234-235: Verify edge cases and consider making the fallback more explicit.
The changes correctly handle assets without chunks, but there are a few points to consider:
- The length comparison for
hasChunkFiltersnow includes the NO_CHUNK_FILTER, which is correct but could be more explicit - The fallback to NO_CHUNK_FILTER could be made more readable
Consider these improvements:
// List of chunks ids, including the NO_CHUNK
const filtersChunkIds = [NO_CHUNK_FILTER, ...chunkIds];
+// Track which filters are enabled
filtersChunkIds.forEach((chunkId) => {
if (filters[`${ASSET_CHUNK}.${chunkId}`]) {
checkedChunkIds.push(chunkId);
}
});
const hasChunkFilters =
checkedChunkIds.length > 0 && checkedChunkIds.length !== filtersChunkIds.length;
-const rowRunsChunkIds = item?.runs?.map((run) => run?.chunkId || NO_CHUNK_FILTER) || [];
+const rowRunsChunkIds = item?.runs?.map((run) => (
+ // Explicitly mark assets without chunks
+ run?.chunkId === undefined ? NO_CHUNK_FILTER : run.chunkId
+)) || [];Let's verify the changes handle all edge cases:
Also applies to: 237-244, 268-269
Summary by CodeRabbit