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

Separated ecmascript-compiler from ecmascript #6824

Closed
wants to merge 1 commit into from

Conversation

mitar
Copy link
Contributor

@mitar mitar commented Apr 15, 2016

Addressing the regression in Meteor 1.3. Meteor 1.3 implies modules when using ecmascript, but previously this was not so. This prevent compiling .js files using ecmascript package when you do not want to use modules at the same time.

By splitting compiler into its own package one can use just the compiler part and not also the modules part.

Fixes: #6680

@mitar
Copy link
Contributor Author

mitar commented Apr 15, 2016

@benjamn, it would be great if this could go into 1.3.2.

@mitar
Copy link
Contributor Author

mitar commented Apr 18, 2016

Pleaseee, merge this.

@mitar
Copy link
Contributor Author

mitar commented Apr 20, 2016

@benjamn: or 1.3.3. I have https://github.com/peerlibrary/meteor-util package being blocked on that and some apps cannot migrate to 1.3 without this.

@mitar
Copy link
Contributor Author

mitar commented Apr 28, 2016

Please!

@mitar
Copy link
Contributor Author

mitar commented May 4, 2016

But really, the most important is this one, @benjamn.

@benjamn
Copy link
Contributor

benjamn commented May 8, 2016

I think what you're really getting at here is that you want a version of ecmascript that doesn't compile modules or imply the modules package. If that's correct, please reorganize this pull request so that there's an ecmascript-no-modules package, and ecmascript depends on that but adds support for modules. Specifically, the ecmascript-no-modules package should not compile import and export statements, whereas this pull request still compiles module syntax but does not provide any runtime support for them.

I'm also going to check out the reproduction for #6680, because I suspect this solution is overkill…

@benjamn
Copy link
Contributor

benjamn commented May 8, 2016

Thanks for digging into this problem, but I believe it's fixable on peerlibrary:util's side, which makes me reluctant to split the ecmascript package in this way. Closing in favor of peerlibrary/meteor-util#3.

@benjamn benjamn closed this May 8, 2016
@mitar
Copy link
Contributor Author

mitar commented May 8, 2016

As I wrote in the pull request you made (and thank you for it!) I do not find the switch to NPM module you are proposing acceptable. It really changes compatibility of client and server side and I do not want to chase bugs because one implementation is different than another (require util vs. require meteor/util vs client-side).

I do not see what is problem with accepting this pull request? It is backwards compatible, it nicely splits compiler from runtime, which was also something done for Blaze (#5903).

whereas this pull request still compiles module syntax but does not provide any runtime support for them.

I do not understand this, if it still compiles module syntax, why it does then work for me? Because my code uses require (which is not module syntax), but not import (which is)?

@benjamn
Copy link
Contributor

benjamn commented May 8, 2016

Commented in the pull request, but also summarizing here: you can use whatever implementation of the util module that you like. The important thing is that you find a way to avoid implementing your own version of the require function, and I think that may be a lot easier than you realize?

Until we exhaust that line of investigation, I have to be honest with you: Meteor is not going to provide a version of the ecmascript package that doesn't support modules. Modules are not an optional part of the ECMAScript specification.

If you really really want to, you are also welcome to fork the ecmascript package. Most of its implementation is in the packages it uses (especially babel-compiler and ecmascript-runtime), so you wouldn't be asking your users to swallow some huge alternative plugin, but it would be your responsibility to maintain it (not Meteor's).

@mitar
Copy link
Contributor Author

mitar commented May 8, 2016

The important thing is that you find a way to avoid implementing your own version of the require function, and I think that may be a lot easier than you realize?

If I understand correctly, your easier proposals are: use unmaintained NPM module, or copy node.js file over and modify it? Any other I missed?

Meteor is not going to provide a version of the ecmascript package that doesn't support modules.

Doesn't my pull request here support modules? You were suggesting to remove them. :-)

I have nicely split it into compiler and runtime. And then I just provide an "alternative runtime" which has a bit special require.

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.

None yet

2 participants