-
Notifications
You must be signed in to change notification settings - Fork 26
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
Non-identifier exports not detected? #47
Comments
Wait, maybe it doesn't have anything to do with keywords. It looks like mapping to module.exports = {fi, if: 0}
{ exports: [ 'fi' ], reexports: [] }
module.exports = {fi, if: !0}
{ exports: [ 'fi' ], reexports: [] }
module.exports = {fi, if: -0}
{ exports: [ 'fi' ], reexports: [] }
module.exports = {fi, if: (true)}
{ exports: [ 'fi' ], reexports: [] }
module.exports = {fi, if: [true]}
{ exports: [ 'fi' ], reexports: [] }
module.exports = {fi, if: null}
{ exports: [ 'fi', 'if' ], reexports: [] }
module.exports = {fi, if: true}
{ exports: [ 'fi', 'if' ], reexports: [] } I guess this is because the grammar is specified as I will use |
@evanw this is just a very simple lexer designed to be small and fast and it doesn't even have an expression parser. We could extend the grammar to support direct primitives in the object form, but for now I'm concerned that may be even more confusing too, it's not a clear line by any means yes. See also #6. Supporting numbers and strings would be straightforward, I'm just not sure we want to push the blurry line in that direction here, would value your thoughts. |
The latest update has been upstreamed now, and the object form detection remains the same without string support. I'm open to further suggestions here, although the window is closing on the current set of updates. Closing for now but just let me know if we should reopen or make changes here further. |
Ah, this makes sense. I was mainly just surprised that support for this wasn't necessary to handle modules like that in the wild. I recently shipped support for the |
Node attempts to make CommonJS exports available as named exports when imported by ES modules. It does this by statically analyzing the CJS module using `cjs-module-lexer`. However, as the name implies, it's a lexer, not a full parser, and it only supports limited common patterns for building a `module.exports` object. Notably, it supports `module.exports = { ... }`, but only if the values in that object are all simple identifiers. Otherwise it will include only the keys up to the first non-identifier value it finds, and bail after that. nodejs/cjs-module-lexer#47 That means that the `Generator` and `Wildcard` exports were missing in ES modules. This change makes them available again.
Node attempts to make CommonJS exports available as named exports when imported by ES modules. It does this by statically analyzing the CJS module using `cjs-module-lexer`. However, as the name implies, it's a lexer, not a full parser, and it only supports limited common patterns for building a `module.exports` object. Notably, it supports `module.exports = { ... }`, but only if the values in that object are all simple identifiers. Otherwise it will include only the keys up to the first non-identifier value it finds, and bail after that. nodejs/cjs-module-lexer#47 That means that the `Generator` and `Wildcard` exports were missing in ES modules. This change makes them available again.
Node attempts to make CommonJS exports available as named exports when imported by ES modules. It does this by statically analyzing the CJS module using `cjs-module-lexer`. However, as the name implies, it's a lexer, not a full parser, and it only supports limited common patterns for building a `module.exports` object. Notably, it supports `module.exports = { ... }`, but only if the values in that object are all simple identifiers. Otherwise it will include only the keys up to the first non-identifier value it finds, and bail after that. nodejs/cjs-module-lexer#47 That means that the `Generator` and `Wildcard` exports were missing in ES modules. This change makes them available again.
I came across some unexpected behavior:
Expected:
Observed:
I expected this to work because ESM lets you do this:
The text was updated successfully, but these errors were encountered: