-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
fix: align dynamic import detection #20115
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
fix: align dynamic import detection #20115
Conversation
/ecosystem-ci run |
commit: |
📝 Ran ecosystem CI on
✅ ladle, marko, quasar, rakkas, astro, react-router, vite-plugin-react, vuepress, vite-environment-examples, vite-plugin-pwa, vitepress, unocss, storybook, vite-setup-catalogue, vite-plugin-cloudflare, sveltekit, vite-plugin-vue, vite-plugin-svelte |
It seems that #19936 will cause some regressions in the ecosystem. For code like this one https://github.com/wakujs/waku/blob/16f21c7ce460071392e995a634acc058a7c5ed53/packages/waku/src/lib/plugins/patch-react-refresh.ts#L14 that calls |
Yeah, I think we cannot avoid these. |
/ecosystem-ci run |
📝 Ran ecosystem CI on
✅ ladle, laravel, qwik, nuxt, rakkas, react-router, quasar, marko, vitepress, vite-plugin-svelte, vite-setup-catalogue, vite-plugin-vue, vitest, vite-plugin-react, unocss, storybook, waku, vite-plugin-pwa, vuepress, vite-environment-examples |
vike is failing with a different error but the reason is same with #20080 (comment) and not caused by this PR. |
## [1.0.0-beta.19] - 2025-06-22 ### 🚀 Features - support `OutputOptions#manualChunks` (#5037) by @hyf0 - advanced-chunks: support `advancedChunks#gruop#name` to be function (#5035) by @hyf0 - rolldown_plugin_import_glob: align with `vitejs/vite#20163` (#5034) by @shulaoda - rust/advanced-chunks: support `MatchGroup#name` to be dynamic (#5033) by @hyf0 - rolldown_plugin_build_import_analysis: align with `vitejs/vite#20117` (#5027) by @shulaoda - rolldown_plugin_build_import_analysis: align with `vitejs/vite#20115` (#5020) by @shulaoda - add validation warning for advanced chunks options without groups (#5009) by @sapphi-red ### 🐛 Bug Fixes - moduleInfo is not updated when entry module is emitted by this.emitFile (#5032) by @IWANABETHATGUY - preserveEntrySignatures: false generates circular imports that hangs with TLA (#5029) by @IWANABETHATGUY - rolldown_plugin_build_import_analysis: align pure dynamic import handling with rolldown-vite (#5016) by @shulaoda - plugin/vite-resolve: normalize leading slash (#5013) by @sapphi-red - debug: `build_id` doesn't increase (#5015) by @hyf0 - side effects in this.emitFile({ type: 'chunk' }) is removed when preserveEntrySignatures: false is set (#5012) by @IWANABETHATGUY ### 🚜 Refactor - rolldown_utils: simplify `block_on_spawn_all` (#5019) by @shulaoda - use `rolldown_utils::futures::block_on` for `WatcherImpl#start` (#5018) by @shulaoda ### 📚 Documentation - jsdoc: document `experimental.attachDebugInfo` (#5028) by @hyf0 - clarify that `advancedChunks` options are in bytes (#5022) by @sapphi-red - add a note that sequential conversion may break the code (#5024) by @sapphi-red ### ⚙️ Miscellaneous Tasks - infra: clean up `dist` before building `rolldown` (#5036) by @hyf0 - Align status notice in readme with documentation (#5021) by @rijkvanzanten ### ❤️ New Contributors * @rijkvanzanten made their first contribution in [#5021](#5021) Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
Description
According to #2519 and e6f7fba#diff-c21c6724d412c50eabaaaf67a8fa37cea7ab889f0c23e628713893f7dd34770a, it seems the
isInNodeModules(importer) &&
condition was to skipimport.meta.glob
processing in deps.But now that
import.meta.glob
processing is done in a separate plugin, this condition no longer made sense.The current condition is skipping edge case dynamic imports only for deps (example cases of them - es-module-lexer repl).
I think the current code is difficult to understand the intent and also the behavior is strange. I think no one would write these in the source code, so I guess we can just remove this condition. If we should support these, we can update the
dynamicImportPrefixRE
regex to/import\s*[(/]/
.