Skip to content

perf: Use filter to improve plugin performance with rolldown #1787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

PatrykKuniczak
Copy link
Contributor

Overview

I've adapt filters instead of making if statements and handling it by JS, now it should be handled by rust and make WXT more amazing ❤️

Manual Testing

Let's run pnpm test and check if everything works fine :)

Related Issue

This PR closes #1716

Copy link

netlify bot commented Jul 2, 2025

Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit 43671bf
🔍 Latest deploy log https://app.netlify.com/projects/creative-fairy-df92c4/deploys/689eec77b38f700008a583c0
😎 Deploy Preview https://deploy-preview-1787--creative-fairy-df92c4.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@PatrykKuniczak PatrykKuniczak changed the title Feat/Use filter for rolldown plugin #1716 feat: Use filter for rolldown plugin #1716 Jul 2, 2025
Copy link

codecov bot commented Jul 11, 2025

Codecov Report

Attention: Patch coverage is 87.13450% with 22 lines in your changes missing coverage. Please review.

Project coverage is 81.13%. Comparing base (7c783fe) to head (5237e20).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...src/core/builders/vite/plugins/extensionApiMock.ts 0.00% 14 Missing ⚠️
...src/core/builders/vite/plugins/resolveAppConfig.ts 66.66% 6 Missing ⚠️
...src/core/builders/vite/plugins/devHtmlPrerender.ts 95.12% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1787      +/-   ##
==========================================
+ Coverage   81.04%   81.13%   +0.08%     
==========================================
  Files         130      130              
  Lines        6649     6723      +74     
  Branches     1091     1075      -16     
==========================================
+ Hits         5389     5455      +66     
- Misses       1249     1257       +8     
  Partials       11       11              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@PatrykKuniczak
Copy link
Contributor Author

Small mistake of me, i've done id checking the other way round, let's run CI one more time, should work that time :)

Copy link

@sm17p sm17p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, seems like a safe code change. Nice work!

@PatrykKuniczak
Copy link
Contributor Author

@sm17p Thanks for your review, now code is a way simpler <3

@aklinker1 aklinker1 changed the title feat: Use filter for rolldown plugin #1716 perf: Use filter to improve plugin performance with rolldown #1716 Aug 3, 2025
@aklinker1 aklinker1 changed the title perf: Use filter to improve plugin performance with rolldown #1716 perf: Use filter to improve plugin performance with rolldown Aug 3, 2025
Copy link
Member

@aklinker1 aklinker1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for figuring this out. Couple of comments, not sure if we need to use regex for exact matches.

@PatrykKuniczak
Copy link
Contributor Author

@aklinker1 As you can see it doesn't accept strings, i've wanted to do it in more simply way, but it isn't possible.

I had plan to check issues and if not open, open it on rolldown repo, but i haven't time, i'm able to do it now -_-

@PatrykKuniczak
Copy link
Contributor Author

PatrykKuniczak commented Aug 15, 2025

@aklinker1 That's in some kind of FAQ:
id is treated as a glob pattern when you pass a string, and treated as a regular expression when you pass a RegExp. In the resolve hook, id must be a RegExp. strings are not allowed. This is because the id value in resolveId is the exact text written in the import statement and usually not an absolute path, while glob patterns are designed to match absolute paths.

In the very bottom of:
https://rolldown.rs/guide/plugin-development#plugin-hook-filters

And i haven't understand it but Perplexity helps me 😆
And that make sens, why Regexp is necessary for resolveId for rest i'm able to simplify it, but i need to check if it'll be working well, if tests pass.

@PatrykKuniczak PatrykKuniczak force-pushed the feat/use-filter-rolldown-#1716 branch from 2714ff4 to 43671bf Compare August 15, 2025 08:14
@PatrykKuniczak
Copy link
Contributor Author

@aklinker1 Should be all good right now, let's approve workflow :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Rolldown] Use filter pattern for Vite hooks
4 participants