-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
refactor: remove acorn #16238
refactor: remove acorn #16238
Conversation
Run & review this pull request in StackBlitz Codeflow. |
import { findStaticImports, parseStaticImport } from 'mlly' | ||
import { parseAst } from 'rollup/parseAst' | ||
import type { StaticImport } from 'mlly' | ||
import { ESM_STATIC_IMPORT_RE, parseStaticImport } from 'mlly' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
findStaticImports
uses acorn
internally, but for our use case, it was not needed.
https://github.com/unjs/mlly/blob/e5e239dea49828723879e63301f6cec5a36ee5d5/src/analyze.ts#L97-L102
https://github.com/unjs/mlly/blob/e5e239dea49828723879e63301f6cec5a36ee5d5/src/analyze.ts#L409-L432
const end = | ||
findCorrespondingCloseParenthesisPosition( | ||
cleanCode, | ||
start + match[0].length, | ||
) + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous implementation relied on acorn to find the end position of import.meta.glob(
. In this new implementation, it finds the close parenthesis of import.meta.glob(
instead.
/ecosystem-ci run |
📝 Ran ecosystem CI on
✅ analogjs, astro, histoire, ladle, laravel, marko, previewjs, quasar, qwik, rakkas, remix, sveltekit, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-pages, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest |
📝 Ran ecosystem CI on ✅ analogjs, astro, histoire, ladle, laravel, marko, nuxt, previewjs, quasar, qwik, rakkas, remix, sveltekit, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-pages, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactor!
The flaky CI fails is happening in main too. I'm investigating this and will submit a fix later. Merging this for now. |
Description
Rollup v4 doesn't use acorn and exposes
parseAst
. This PR removes acorn by using that function instead.Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).