Skip to content
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: read config files from ESM projects, changes require to dynamic import #8726

Conversation

josefaidt
Copy link

↪️ Pull Request

Migrates usage of dynamic require to dynamic import in order to resolve issues reading config files from ESM projects (i.e. projects with "type": "module" in the root package.json)

💻 Examples

(from the linked issue)

  1. pnpm create plasmo --with-svelte
  2. add "type": "module" to project root's package.json
  3. rewrite svelte.config.js to ESM (i.e. ... export default {}
  4. run pnpm build
  5. observe error
➜  pnpm build

> [email protected] build /Users/josef/Documents/playground/plasmo-svelte/ban-maniacal-falcon
> plasmo build

🟣 Plasmo v0.60.2
🔴 The Browser Extension Framework
🔵 INFO   | Prepare to bundle the extension...
🔴 ERROR  | require() of ES Module /Users/josef/Documents/playground/plasmo-svelte/ban-maniacal-falcon/svelte.config.js from /Users/josef/Documents/playground/plasmo-svelte/ban-maniacal-falcon/plasmo/node\_modules/.pnpm/@[email protected]/node\_modules/@parcel/utils/lib/index.js not supported.
Instead change the require of svelte.config.js in /Users/josef/Documents/playground/plasmo-svelte/ban-maniacal-falcon/plasmo/node\_modules/.pnpm/@[email protected]/node\_modules/@parcel/utils/lib/index.js to a dynamic import() which is available in all CommonJS modules.

🚨 Test instructions

I created a few tests with proper inputs and ran the tests directly with mocha (yarn test didn't work for me)

yarn mocha packages/core/utils/test/config.test.js

Happy to change the test descriptions and input setup

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@devongovett
Copy link
Member

Unfortunately, this will break caching. The issue is the same as with ESM plugins (#7639). Parcel relies on being able to monkey patch require in order to track all the files that get loaded, which is used to invalidate the cache. With ESM, there is no (stable) way of doing this in Node yet. In this case, that means if the config had any dependencies (e.g. imports), Parcel wouldn't know about them and if they changed would not invalidate the cache. Until we can solve that, it's best to use .cjs files for configs if you have "type": "module" in your package.json.

@mischnic
Copy link
Member

Will be fixed by #8913

@mischnic mischnic closed this Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants