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

literal exports not supported #6

Open
FredKSchott opened this issue Sep 30, 2020 · 7 comments
Open

literal exports not supported #6

FredKSchott opened this issue Sep 30, 2020 · 7 comments
Labels
edge case wontfix This will not be worked on

Comments

@FredKSchott
Copy link
Contributor

As always, awesome work on this library, @guybedford :)

I'm integrating this into Snowpack to match Node.js support, and was surprised to see this limitation:
https://github.com/guybedford/cjs-module-lexer/blob/9f32d7a83a435ed6f7716318a5367a82d6480079/test/_unit.js#L317-L320

I assumed many packages would rely on this form. Is this feasible because most ESM->CJS compilers don't actually use this form in practice? Is there any support for this planned? Not sure that I'll have time to work on this myself (still some work left to do on the Snowpack integration) but mainly just curious on the background and how easy this would be to implement.

@FredKSchott FredKSchott changed the title literal exports not scanned literal exports not supported Sep 30, 2020
@guybedford
Copy link
Collaborator

Hi @FredKSchott, thanks for the feedback here.

Do you have an exact example of the sort of code pattern you'd like to see supported? Arbitrary expressions can't be supported as this is not a parser. But specific expression types might be possible.

More info on this in https://github.com/guybedford/cjs-module-lexer#exports-object-assignment.

@FredKSchott
Copy link
Contributor Author

Ah, maybe this is a regression then. In the docs you have:

// DETECTS EXPORTS: a, b, c
module.exports = {
  a,
  b: 'c',
  c: c
};

But when I run this with the lexer, I get:

{exports: ["a"], reexports: []}

That seems to contradict the comment in the code snippet.

@guybedford
Copy link
Collaborator

Thanks, that was actually a readme typo - fixed in 4d56d33.

Would exactly supporting strings on the RHS specifically help in your use case? I didn't think this was such an important feature but they are a good example of a case that is easy to add as an extension. Again an example would help to understand what would be useful.

@FredKSchott
Copy link
Contributor Author

no real-world use-case on my end, this came out of a simple test file that I'd written to test that it was working correctly. If I see one, I'll definitely let you know.

We'll be moving forward with this in Snowpack as our default-on CJS handling, so hopefully our community surfaces some good feedback as well! Moving this from config opt-in to default-on is very exciting for us, especially for React developers who seem to run into this the most.

@guybedford
Copy link
Collaborator

guybedford commented Oct 1, 2020 via email

@FredKSchott
Copy link
Contributor Author

@guybedford are you looking for popular packages not working? Just ran into chai not working, which I know is a popular project. But they use some very strange export language that even a true parser would have trouble with: https://unpkg.com/[email protected]/lib/chai.js

@guybedford
Copy link
Collaborator

guybedford commented Oct 3, 2020 via email

@guybedford guybedford added the enhancement New feature or request label Oct 29, 2020
@guybedford guybedford added edge case wontfix This will not be worked on and removed enhancement New feature or request labels Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
edge case wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants