meteor / meteor-feature-requests

A tracker for Meteor issues that are requests for new functionality, not bugs.
Other
89 stars 3 forks source link

Meteor should transpile js in node_modules #6

Closed mwarren2 closed 6 years ago

mwarren2 commented 7 years ago

Migrated from: meteor/meteor#8640

trusktr commented 7 years ago

Maybe this can be somehow configurable. There are indeed packages that ship original source files written in ES6 module format (and using new language features). These packages usually also include transpiled "dist" files.

One advantage of having this is that if someone wants to make an NPM package just for Meteor, they don't need to worry about setting up Webpack/Babel/etc, they can just write code, publish it, say that it works in Meteor, and forget about it, without having to worry about tooling... because Meteor already has all the tooling.

ejfrancis commented 7 years ago

Perhaps we could do something similar to what rollup does where you can specify a "module" main in your package.json which is a non-transpiled es2015 module?

joncursi commented 7 years ago

I hit this issue ~50% of the time I install a new module from npm, which forces me to hunt for an alternate version to specifically make Meteor happy. Other projects I'm working on (based on Webpack, React Native, etc.) run these node_modules without a hitch. It's starting to get pretty painful.

It seems like a lot of modules, especially those in the react-native-* world (i.e. react-native-vector-icons, react-native-elements) are using more advanced JS standards, and Meteor doesn't know what to do with it. Browser console throws:

Uncaught SyntaxError: Unexpected token import

What can we do to fix this? Can Meteor do some additional transpilation on the bundle before sending to the browser?

benjamn commented 7 years ago

@joncursi So those npm packages don't work in any version of Node without transpilation? That seems like a strange choice for them to be making!

While I can certainly feel your pain, you're attempting (if I understand your project) to reuse code designed to work specifically with React Native in Meteor projects. I would love to make that easier, if possible, but the rate at which you encounter this problem is definitely not representative of most of the npm ecosystem, nor of most Meteor apps. Just something to bear in mind.

benjamn commented 7 years ago

@ejfrancis The module field is supposed to identify to code that has been completely transpiled except for import and export declarations, so that idea might help if we started transpiling just import and export in node_modules.

The last time I looked into that, the problem was that other CommonJS modules from npm don't know how to deal with what's exported by the ECMAScript module source tree (specifically the .default property). And of course using the main entry point for CommonJS but the module entry point for ECMAScript is a bad idea because then you get two different copies of the package depending on who's importing it, with different shared state.

Using the module field would not help with packages using syntax targeting Node versions more recent than Node 4, such as async/await. I think Meteor 1.6 (Node 8) will be a better solution to that problem than transpilation, in the long term.

trusktr commented 7 years ago

I think Meteor 1.6 (Node 8) will be a better solution to that problem than transpilation, in the long term.

But there's also client code and supporting older browsers. It may be possible some NPM module is designed for browsers, but the code published is for too recent of a browser than supported by the app dev.

What about some sort of configuration json file that Meteor modules/ecmascript can read, that defines certain node modules that should be transpiled just like other non-node_modules code in the app?

benjamn commented 7 years ago

@trusktr That's a good point. I think the only configuration we should need is the "engines" field of the package's package.json file.

trusktr commented 7 years ago

That would be ideal if package authors were willing to put certain fields in package.json. The reality is: it is unrealistic to expect that every package author will put every required field for every possible target environment into package.json.

It would be great for a Meteor app dev to be able to apply a setting without relying on a package dev having to do it, and without having to fork a package just to add the setting.

benjamn commented 7 years ago

I'm comfortable saying that you (an npm package author) have to specify engines if you want any special treatment of new syntax. In most cases that I've seen where an npm package uses syntax not supported by the current LTS version, there's an appropriate engines field in package.json, and it is a perfectly reasonable thing to ask the package author to change.

mwarren2 commented 7 years ago

@benjamn After reading your technical comments, which are a little beyond me, I hope that my use case, which I detailed in the original issue before turning it into a feature request, is still relevant.

Just to sum up, my meteor builds are failing when trying to install quasar-framework (a vue-based ui framework) from npm. The .js files in the quasar-framework /dist folder have been converted and have no imports. The build is failing on imports from .vue files while apparently compiling from the /src folder. There's an example test case in the original post.

If I'm barking up the wrong tree by introducing the problem in this feature request please set me right.

ghybs commented 7 years ago

Hi @mwarren2,

I am not sure how just npm installing quasar-framework makes your example app fail, without even trying to import it (maybe that is how the Atmosphere package akryum:vue works?)

Anyway, there is a demo Meteor boilerplate from Quasar Framework: https://github.com/quasarframework/quasar-template-meteor

If I understand correctly, for now they have just copied their es6 dist file into the imports folder and you then import it on the Client.

Hope this helps.

mwarren2 commented 7 years ago

@ghybs Thanks. However the Meteor boilerplate at quasar-template-meteor is mine, I've been working for a while with quasar-framework's creator to get it working with Meteor from npm.

trusktr commented 7 years ago

Just an idea: after NPM installing stuff, use a postinstall script to symlink the needed file(s) into an imports folder. Then it can at least be automatic that way, for now.

On Jun 25, 2017 11:38 PM, "mwarren2" notifications@github.com wrote:

@ghybs https://github.com/ghybs Thanks. However the Meteor boilerplate at quasar-template-meteor is mine, I've been working for a while with quasar-framework's creator to get it working with Meteor.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/meteor/meteor-feature-requests/issues/6#issuecomment-310975537, or mute the thread https://github.com/notifications/unsubscribe-auth/AASKzp0R31nHu7z9MLKYB1VJh7iYo0Vzks5sH1H4gaJpZM4Nxxr2 .

mwarren2 commented 7 years ago

@trusktr, @benjamn Ah, it appears there is a solution to my particular problem, from akryum:vue-component

The solution is here and is as follows:

Add a .vueignore file in the project root with the following content:

node_modules/

I can now build my test app, so my case is solved.

aogaili commented 7 years ago

I've ran into many issues with NPM packages using ES6 syntax when running the app on older browsers.

Any workaround for this?

lukejagodzinski commented 6 years ago

@benjamn today I wrote a post on Meteor forums about things related to this topic.

I think that Meteor should transpile some of the packages in the node_modules directory if they contain ES imports/exports. The good example of a package that is using ES imports/exports is lodash-es. We should try to embrace new ES features and encourage people to switch to the new syntax. Currently it's also possible to use ES imports/exports in Node thanks to @std/esm.

In essence, I wouldn't want to wait until 2018 (or even 2020) when ES import/export syntax will be available in Node and all browsers

benjamn commented 6 years ago

To be completely honest, the main reasons we don't currently compile modules in node_modules are (1) performance (it would take time, obviously), and (2) back in the Meteor 1.2 days, when ecmascript was first introduced, it seemed reasonable to assume that packages published to npm would be compiled down to ES5 first.

We've always known that this assumption would become less and less reasonable over time, as package authors began to publish newer syntax to npm, and at some point we would have to start compiling code from npm, at least for the browser. Node 8 handles almost everything natively (except for ECMAScript module syntax), but that only helps on the server.

Now that Meteor 1.6 is out, I think this issue deserves top priority for Meteor 1.6.1, which is why I put it in the milestone. Even compared to the other feature requests currently in the milestone, I think this feature is by far the most important.

At the very least, we have to start compiling any modules that we include in the client bundle. This is complicated for some obscure legacy reasons, but that doesn't matter. We can rewrite the problematic code from scratch, if necessary.

Ideally, I would love to reconsider the dialect of ECMAScript that compiler plugins in Meteor (not only ecmascript but also coffeescript, barbatus:typescript, etc.) should be targeting when generating compiled code. Right now it's ES5 + CommonJS require and exports. I would be much happier if that also included import and export syntax, because then compiler plugins would no longer be responsible for compiling that syntax, and we could implement fancy optimizations like tree shaking in Meteor core, rather than forcing plugins to understand inter-module dependencies.

Performance remains a concern, but Node 8 is noticeably faster, Babel is getting faster all the time, and of course we can cache the compiled code aggressively.

Thanks to everyone for keeping this issue active. I look forward to making progress on this soon, because I agree it's long overdue.

lukejagodzinski commented 6 years ago

@benjamn great to hear that! I think it's definitely the right time to do it. I wouldn't be concerned a lot about performance because as you said we can always make use of the cache. The initial load might take more time but later it will be rather not very common to recompile everything. I would only be worried about tree shaking implementation but Rollup does it (or at least it's what they say in documentation).

If it goes about compilers, I see that for example, the typescript compiler has an option to customize target ES version per project https://github.com/barbatus/typescript#es6 that way developers can just choose.

benjamn commented 6 years ago

By the way, here are my thoughts on tree shaking in Meteor: https://github.com/meteor/meteor-feature-requests/issues/206#issuecomment-341117517

yorrd commented 6 years ago

@benjamn just for the record: I'm really glad you're on this issue right now. I'm integrating Polymer into Meteor (which works like a f***n charm since Polymer uses es6 modules (I love you for moving along with es6 imports (nested brackets suck))). The issue is, I can't import the Polymer library or its components on the client side, because they would have to be compiled. When copying to the imports, of course, it works fine.

Just another use case for this, thanks for your work! Please let me know if I can do anything, maybe test an alpha :)

robinknox commented 6 years ago

Another use case for this is the react-facebook NPM package which currently contains some ES6 https://github.com/seeden/react-facebook meaning that I can't currently use it on the clientside.

trusktr commented 6 years ago

@benjamn

I am assuming that nothing in node_modules will be compiled by default, and that this new feature will be an opt-in in feature.

Have you already come up with some ideas on what the API will look like for opting into transpiling certain packages in node_modules?

trusktr commented 6 years ago

Would a it make sense to have an option for specifying that transient dependencies of a targeted package should also be transpiled?

Manually having to specify various dependency packages of a main package the the app depends on might get tedious, but it would lead to the best performance. In many cases, just specifying for Meteor to transpile a package's whole dependency tree would be easier and perfectly fine if the build time difference is tolerable.

dbx834 commented 6 years ago

Here is another use-case:

Meteor doesn't work so well at the moment with Box, specifically box-ui-elements and box-react-ui.

Maybe it's a good idea if it can be controlled what packages in node_modules will be transpiled rather than transpiling everything...

PS: I've put in a request to them to include ES5 in their distribution - https://github.com/box/box-ui-elements/issues/184.

benjamn commented 6 years ago

We think we've finally found a way to solve this problem: https://github.com/meteor/meteor/pull/9771

tl;dr Use symbolic links to expose node_modules code within your application, outside of node_modules. Meteor will compile the exposed code as if it was part of your application, using whatever compiler plugins you have installed, and also guarantee that you get the compiled code when you import from node_modules (this is they key new feature).

CaptainN commented 6 years ago

That feature looks sweet!

Can we make it even easier? For example - a set of config options in package.json (in the meteor tree) that would allow a developer to cherry pick the packages they want without having to create local symlinks or specifically clone a repo? We could even allow a basic set of package reconfiguration (to allow the user to select from the src directory instead of dist). Something like:

{
  "meteor": {
    "mainModule": {
      "client": "client/main.js",
      "server": "server/main.js"
    },
    "modernModules": {
      "package-name": {
        "main": "src/main.js"
      }
    }
  }
}

Basically, that setting could create the symlinks from config at compile time, instead of requiring the user to set those up (may be easier especially on Windows).

benjamn commented 6 years ago

@CaptainN Honestly I think the main use case will be cloning the package repository locally and then running npm install path/to/package to link it into node_modules, which doesn't require the developer to make any explicit symbolic links.

I would rather not invent new configuration options that are less powerful than the "real" way of doing this, which may involve modifying the package locally, adding your own .babelrc file, etc. When you start to think about all the possible dimensions of these modifications, coming up with a package.json-based configuration system that does something equivalent (and teaching everyone how to use it) feels pretty daunting.

That said, if folks really need the new functionality to be simpler, we can maybe consider implementing another form of configuration in the future.

CaptainN commented 6 years ago

@benjamn That's true. It's actually similar to how I manage certain Meteor packages.

I wonder whether there are enough packages in npm that could benefit from simply using the ES6+ modules directly (and getting the benefits of Meteor's modern/legacy builds) rather than always using what are essentially legacy builds in the main prop/dist folders of many npm packages.

You are probably right though - it may not be as simple in most cases as defining the entry point in an existing npm module, which is the extent of flexibility I was thinking about with regard to configuration (basically, an alternative to symlinks, with one additional config option).

mwarren2 commented 6 years ago

@benjamn My original reason for opening this issue was due to problems with quasar-framework, which I had to transpile manually, and this has now been solved.

meteor/meteor#9771 has elegantly dealt with the problem, which I solved by linking to the node_modules installation from the /imports folder, and employing setMinimumBrowserVersions(), as shown in the documentation for Meteor 1.7 .

Thank you for all your work on this.

As far as I am concerned this issue can be closed.

benjamn commented 6 years ago

@mwarren2 That's great to hear! Feel free to comment here in the future if you run into any problems with this workflow.

yanickrochon commented 4 years ago

I have run into this problem with a few npm packages lately, big packages like TypeORM and BabylonJS, etc. With JS features growing, and the general acceptance of import/export and other ES6+ features, it seems like holding back on this seems a hacky approach.

I would love to have an option to manually specify which npm module should be included in the build process. I don't know, something like .meteor.conf such as

{
   "build": {
      "includeModules": [
         "typeorm",
         [ "@babylonjs/core", { "target": [ "browser" ] } ]
      ]
   }
}

Some packages, such as TypeORM does not have an already transpiled release, and this makes for a hacky solution, as documented earlier.

As I have other projects using react-scripts, which does compile node_modules with no issue, this is a problem that can be fixed. As other package maintainers opine, it is not the package owners to decide how their packages should be compiled, and with what compatibility layers it should have; each project is different, and each project should optimize transpiling for their own needs.