refactor(utils): Extract packages - use forEach instead of reduce #4288
Conversation
#11313 Bundle Size — 366.46KiB (0%).b167eed(current) vs 4d71538 master#11311(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
| Current #11313 |
Baseline #11311 |
|
|---|---|---|
 Initial JS |
323.81KiB |
323.81KiB |
| Initial CSS |
42.65KiB |
42.65KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
3 |
3 |
| Assets |
4 |
4 |
| Modules |
668 |
668 |
| Duplicate Modules |
0 |
0 |
| Duplicate Code |
0% |
0% |
| Packages |
36 |
36 |
| Duplicate Packages |
1 |
1 |
Bundle size by type no changes
| Current #11313 |
Baseline #11311 |
|
|---|---|---|
| JS |
323.81KiB |
323.81KiB |
| CSS |
42.65KiB |
42.65KiB |
Bundle analysis report Branch refactor-package-extraction Project dashboard
Generated by RelativeCI Documentation Report issue
vio
force-pushed
the
refactor-package-extraction
branch
from
6bdd18b to
b167eed
Compare
WalkthroughThe pull request introduces a refactoring of the Changes
Poem
Finishing Touches
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 (
|
![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
🧹 Nitpick comments (2)
packages/utils/src/webpack/extract/modules-packages.ts (2)
114-117: Update the comment to accurately reflect the logic.The comment "Same package name, but different paths" is misleading as the code is actually checking for packages with the same path, which could indicate symlinks.
- // Same package name, but different paths (eg: symlinks) + // Check for packages with the same path (eg: symlinks)
131-137: Consider simplifying the max calculation.The max calculation could be simplified using optional chaining and more descriptive variable names.
- const lastIndex = - max( - Object.keys(packages) - .map((id) => id.split('~')) - .filter(([id]) => id === packageMeta.id) - .map(([__, index]) => parseInt(index, 10)), - ) || 0; + const existingIndices = Object.keys(packages) + .map((id) => id.split('~')) + .filter(([id]) => id === packageMeta.id) + .map(([__, index]) => parseInt(index, 10)); + const lastIndex = max(existingIndices) ?? 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/utils/src/webpack/extract/modules-packages.ts(1 hunks)
🔇 Additional comments (3)
packages/utils/src/webpack/extract/modules-packages.ts (3)
82-82: LGTM! Clean initialization with proper type annotation.
84-90: Great refactor! The forEach implementation improves readability.The control flow is now more straightforward with clear early returns for invalid cases. This is a good example of choosing the right tool for the job - forEach is more intuitive than reduce for this use case.
84-146: Verify test coverage for the refactored implementation.While the refactoring improves readability and maintainability, let's ensure all edge cases are still covered by the test suite.
✅ Verification successful
Test coverage verified successfully
The test suite comprehensively covers all scenarios in the refactored implementation, including package identification, value accumulation, path handling, and duplicate package scenarios. The refactoring maintains full compatibility with existing test cases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for related test files fd -e test.ts -e spec.ts modules-packages # Look for test cases covering different package scenarios rg -A 5 "describe.*modules-packages"Length of output: 82
Script:
#!/bin/bash # Search for test files in the webpack/extract directory with various extensions fd -e test.ts -e spec.ts -e test.tsx -e spec.tsx . packages/utils/src/webpack/extract # Search for files importing or using modules-packages rg -l "modules-packages" packages/utils/src # Search for test cases mentioning packages or modules extraction rg -A 5 "describe.*\b(package|module|extract)" packages/utils/srcLength of output: 11133
Script:
#!/bin/bash # Get the content of the main test file cat packages/utils/src/webpack/extract/__tests__/modules-packages.tsLength of output: 11330
Summary by CodeRabbit
reducemethod withforEachloop