Skip to content

fix: exclude acorn.js from node bundle#6141

Closed
TrySound wants to merge 1 commit intovitejs:mainfrom
TrySound:exclude-acorn-js
Closed

fix: exclude acorn.js from node bundle#6141
TrySound wants to merge 1 commit intovitejs:mainfrom
TrySound:exclude-acorn-js

Conversation

@TrySound
Copy link
Contributor

@TrySound TrySound commented Dec 16, 2021

Description

I found vite has bundled both acorn.mjs and acorn.js.
Plugins use it as fallback when parser instance is not passed.

du -ck dist/node
before: 12260 kB
after: 11612 kB

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

I found vite has bundled both acorn.mjs and acorn.js.
Plugins use it as fallback when parser instance is not passed.
@yyx990803
Copy link
Member

Hmm, this would require acorn to be present as a peer dep for Vite and will fail if using strict package managers like pnpm.

@TrySound
Copy link
Contributor Author

@yyx990803 Not really. See https://github.com/acornjs/acorn-class-fields/blob/master/index.js#L6

Acorn already passes own instance when invoking plugins. We don't need to resolve import in plugins.

https://github.com/acornjs/acorn/blob/master/acorn/src/index.js#L56

@yyx990803
Copy link
Member

Ah I see. So the runtime require() actually never gets called. Makes sense 👍

@patak-cat patak-cat added this to the 2.8 milestone Dec 19, 2021
@patak-cat
Copy link
Member

Let's merge this one once we start the 2.8 beta period

@TrySound TrySound mentioned this pull request Dec 27, 2021
9 tasks
@TrySound TrySound closed this Dec 28, 2021
@TrySound TrySound deleted the exclude-acorn-js branch December 28, 2021 07:48
@patak-cat patak-cat removed this from the 2.8 milestone Dec 28, 2021
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.

3 participants