This repository has been archived by the owner on May 22, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat: use esbuild by default for es modules, if detected #625
feat: use esbuild by default for es modules, if detected #625
Changes from 4 commits
a3bce94
28fe10b
491061a
ebc2cd3
6b5bfa3
c362ff8
964a9b6
136daea
aab738e
e512790
542dd74
b39fc49
074c650
ff27067
3f8b2af
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
We already have a JavaScript parser (
acorn
) as a dependency, which should be able to tell us whether a file uses ESM or not. If it is, I would try to use it before adding a new dependency.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.
sure sure, I'll take a look 👍
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.
I am wondering about the performance difference there considering:
acorn
is implemented in JavaScript whilees-module-lexer
is in WebAssemblyacorn
does lexing+parsing whilees-module-lexer
does lexing onlyacorn
parses all possible JavaScript syntax whilees-module-lexer
only looks for imports/exportsThe author of
es-module-lexer
actually claims the following: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.
I second @ehmicky 's evaluation - we need a parser and that's just not what
es-module-lexer
provides.that being said,
es-module-lexer
does have a very light footprint... if we want, I can look into how to accomplish this withacorn
, but we can't replaceacorn
with thisThere 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.
this is no longer a reachable flow, since we'll detect
import
s/export
s and default toesbuild