-
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
literal exports not supported #6
Comments
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. |
Ah, maybe this is a regression then. In the docs you have:
But when I run this with the lexer, I get:
That seems to contradict the comment in the code snippet. |
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. |
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. |
Thanks @fks, the faster you can run a feedback cycle here the better as we
only have a short window before it locks down on Nodejs 12. If there’s
something to fix now we can still get that in but over the next week or so
that changes fast. Appreciated for the investigations!
…On Wed, Sep 30, 2020 at 17:04 Fred K. Schott ***@***.***> wrote:
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.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAESFSSOQOJ4GCORTXH6CHLSIPBSLANCNFSM4R6VFTEA>
.
|
@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 |
Yeah that’s the sort of case we have to draw a line on in not supporting
unfortunately. Maybe if it were an explicit iteration and copy it could be
considered, but the implicit ‘use’ function is definitely outside this.
…On Fri, Oct 2, 2020 at 21:57 Fred K. Schott ***@***.***> wrote:
@guybedford <https://github.com/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:
***@***.***/lib/chai.js
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAESFSQE7XKMLY4LMHKF4BTSI2VKZANCNFSM4R6VFTEA>
.
|
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.
The text was updated successfully, but these errors were encountered: