perf: use hook filters #631
Conversation
WalkthroughThe code refactors the plugin's internal logic to use structured filter objects with explicit include patterns and handler functions for module filtering, resolution, transformation, and loading. Constants are introduced for precomputed file paths and regex patterns, replacing repeated inline computations. The resolveId, transform, and load hooks are restructured into objects with declarative filters and handlers, but their core logic and external API remain unchanged. The auto-export plugin replaces Changes
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
commit: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #631 +/- ##
==========================================
+ Coverage 62.35% 62.46% +0.11%
==========================================
Files 32 32
Lines 3140 3136 -4
Branches 586 586
==========================================
+ Hits 1958 1959 +1
+ Misses 1176 1172 -4
+ Partials 6 5 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 (1)
src/core/moduleConstants.ts (1)
39-47: Consider adding JSDoc comments to the new utility functions.The implementation is correct, but adding JSDoc documentation would improve maintainability by explaining the purpose and parameters of these utility functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/moduleConstants.ts(1 hunks)src/index.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/index.ts
BREAKING CHANGE: if you were **manually** using the AutoExports experimental plugin with, you need to replace the option `filterPageComponents` with `transformFilter`. Otherwise this change doesn't affect you.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/data-loaders/auto-exports.ts (1)
74-74: Remove debug console.log statement.Debug logging should be removed before production deployment.
- console.log(transformFilter)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/core/moduleConstants.ts(1 hunks)src/data-loaders/auto-exports.ts(3 hunks)src/index.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/core/moduleConstants.ts
- src/index.ts
🔇 Additional comments (4)
src/data-loaders/auto-exports.ts (4)
6-6: Import changes align with type requirements.Added import of
StringFiltertype fromunpluginsupports the interface changes below.
42-47: Interface updated to use StringFilter type.Changed from
filterPageComponentstotransformFilterproperty with updated documentation.
67-69: Parameter name updated to match interface changes.Function parameter properly updated from
filterPageComponentstotransformFilter.
78-84: Filter pattern implementation looks correct.The transform hook now uses the structured filter pattern with
id: transformFilter, simplifying the handler function by removing the manual filtering.
| return | ||
| resolveId: { | ||
| filter: { | ||
| id: { |
There was a problem hiding this comment.
can't this just be id: [MODULE_..., MODULE_..., routesRE]?
There was a problem hiding this comment.
I see in the docs https://rolldown.rs/guide/plugin-development#plugin-hook-filters that they need to be regexp although it feels counter intuitive. I wonder if unplugin just aligned with this, the other way around, or simply something els.
There was a problem hiding this comment.
I think if they aren't regexps they are treated as globs. And as you mentioned, rolldown doesn't allow that for resolveId.
Refactor the route handling logic to utilize a hook filter, improving performance and maintainability when using Rolldown as bundler.
This should be in line with what
loadInclude/transformIncludedoes for webpack too.Related issue
e18e/ecosystem-issues#171
Summary by CodeRabbit
Summary by CodeRabbit