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

Generate modern output for dist-esm #349

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Mar 27, 2021

No description provided.

@andywer
Copy link
Owner

andywer commented Mar 27, 2021

Not sure if esnext is maybe a bit much as it's a moving target. Any reason not to target a specific language version, like es2020?

@aminya
Copy link
Contributor Author

aminya commented Mar 27, 2021

Not sure if esnext is maybe a bit much as it's a moving target. Any reason not to target a specific language version, like es2020?

That's exactly the beauty of it. EsNext is always the latest JavaScript, and people who want to run the code on a fixed target will use Babel.

@andywer
Copy link
Owner

andywer commented Mar 27, 2021

Yes, but my point is that this is a library and this configuration determines what we ship to the user. So what are we going to tell the users what minimum ES spec their tool stack needs to support, so that using threads.js won't kill their build?

We would need to tell all users to always make sure that the tools they are using need to support whatever is the bleeding edge in that moment in order to use threads.js…

@aminya
Copy link
Contributor Author

aminya commented Mar 27, 2021

The CI errors are fixed in #351

@andywer
Copy link
Owner

andywer commented Mar 27, 2021

Thanks so much for your help, @aminya! 🥳

PS: Please always share your view on things if you don't agree with me. Not sure what your current thoughts on ESNext vs ES2020 are, but it's important to me personally to have someone challenge what I write here. That kind of feedback is always appreciated 😉

@aminya
Copy link
Contributor Author

aminya commented Mar 28, 2021

Thank you for your attention!

Both work for me, and I don't have a strong opinion. Personally, I think of dist-esm as a build with the latest JavaScript, and those who opt-in to use it will usually use Babel to transpile it based on their target. But it is also possible to bump this version every year manually.

@andywer
Copy link
Owner

andywer commented Mar 29, 2021

I merged #351. If you rebase (or merge into) this PR, I can merge it, too 👍

@andywer
Copy link
Owner

andywer commented Mar 29, 2021

I don't think node.js is ok with ES2020 😉

@andywer
Copy link
Owner

andywer commented Mar 29, 2021

ES2017 is safe.
ES2018 is not safe to use with node 10.

@aminya
Copy link
Contributor Author

aminya commented Mar 29, 2021

This error is not thrown from Nodej, but from eprisma which is used in envify which is used in puppet-run.

The point of #351 was to separate the building phase from the testing phase, so we can use the latest JavaScript. It seems all the tools support the latest JavaScript except puppet-run. Do you want to update its envify dependency?

https://github.com/andywer/puppet-run

This fork (@goto-bus-stop/envify) has updated envify:
https://github.com/goto-bus-stop/envify

@andywer
Copy link
Owner

andywer commented Mar 29, 2021

Ohh, I see. Just merged and published puppet-run.

Man, you are so quick! Thanks for all your efforts 👍 😊

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.

2 participants