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

ADD browserify support #137

Merged
merged 1 commit into from
May 14, 2017
Merged

Conversation

serapath
Copy link
Contributor

for each dependency, browserify only executes transforms that dependency needs.
This speeds up the process a lot, because transforms don't get executed globally on all dependencies.

Therefore each dependency needs to list it's required transforms and make them install so a module that uses that dependency can execute the pipeline and all required transforms have been installed :-)

Would be really nice if this could be merged. thx ;-)

@ChisholmKyle
Copy link

I was getting the browserify parse error: "ParseError: 'import' and 'export' may appear only with 'sourceType: module'" and this fixed it! thanks.

@serapath
Copy link
Contributor Author

yes - i think this pull request fixes that error. I hope it gets merged soon,

@customlogic customlogic changed the base branch from master to develop May 14, 2017 19:28
@customlogic customlogic merged commit 60009ca into dataarts:develop May 14, 2017
@serapath
Copy link
Contributor Author

@ChisholmKyle thx. i found https://www.npmjs.com/package/dat.gui but it seems it's quite outdated

Can a release be cut? npm only has version 0.6.1 but on github it's 0.6.5...

I would like to use this change in production.

@derhuerst
Copy link

the tools added by @serapath should most likely be devDependencies instead of dependencies.

@serapath
Copy link
Contributor Author

hey :-)
...but if they are devDependencies an npm install will not install them. Thus, when you try to run your current project, it will break, because one or more modules will be missing and sometimes that can even cause a little bit arcane error messages.

If you install a dependency which has a dependency which has a dependency which uses a transform... would you expect your your project to break when you try to run something like browserify index.js -t babelfiy > bundle.js ...just because that deeply nested dependencies specifies transforms which havent been installed after npm install because the author did not add them or added them to the devDependencies?

I agree with the theoretic thought, that something that isn't explicitly required by your project should probably be a dev dependency... but then again, i tend to not care how things are called, but what they do.

so dependencies get installed on npm install no matter if its the main module or a dependency,
while devDependencies dont get installed when a module is a depdency... ...so that's how they work and i tend to use things to do what i need.

It's nice that ppl make up theories (e.g optional dependencies), but they figure out it was a mistake and they deprecate or remove or use them differently than was once intended...


If you have a better idea, how would you get around the potential problem of arcane error messages - or even if not arcane - then the need to install maybe a few or even half a dozen depdencies manually using npm install xxx (because this might potentially be caused by quite a few modules in your dependency tree

these days, so many ppl put some unnecessary ES6 into their modules

@derhuerst
Copy link

derhuerst commented May 18, 2017

...but if they are devDependencies an npm install will not install them.

@serapath Let's be a bit more specific here to avoid confusion. npm install will in fact install them, but npm install dat.gui won't.

If you install a dependency which has a dependency which has a dependency which uses a transform...

It doesn't use the transform. Also, I consider transpiling code a build step, therefore it should be a devDependency and the module published to npm should contain both the original src as well as the transpiled code. This seems to be the best practice.

@serapath
Copy link
Contributor Author

serapath commented May 18, 2017

hm, maybe i'm misunderstanding something.

...but if i type npm install when i work on project foo with the dependency bar, it won't install the devDependencies of bar, right?

If the project bar specifies a packages.json#browserify#transform field and i am using browserify in my foo project it fails, because browserify tries to apply the browserify transforms of bar to bar, but they havent been installed.

...and this will or might happen to many modules in my dependency tree - especially if they use ES6.

I am basically forced to run transforms globally and apply them to every single module in my dependency tree - which is slow and sometimes dangerous (maybe it breaks stuff) and i would like to avoid that using package.json#browserify#transform field, which get's picked up by browserify but fails if transforms used by dependencies havent been installed. :/

maybe there is a trick to get them to install without putting them in package.json#dependencies


Maybe i'm completely wrong here. Haven't tried in a while, but the last time i did i remember an npm install did not install devDependencies of a modules dependencies ... i think

@derhuerst
Copy link

...but if i type npm install when i work on project foo with the dependency bar, it won't install the devDependencies of bar, right?

correct.

I agree with you that the situation about transpiling dependencies is tricky. This is my opinion:

I consider transpiling code a build step, therefore it should be a devDependency and the module published to npm should contain both the original src as well as the transpiled code. This seems to be the best practice.

@serapath
Copy link
Contributor Author

I also consider it a build step.
And in theory it should be listed under devDependencies, but in practice this is currently very inconvenient and i run into missing dependencies all the time.

Also - i feel it's wrong to apply "transforms" to every dependency in the dependency tree, which seems to make it slower then necessary and introduces unnecessary risk of breaking things, by applying transforms to stuff that was never meant to have a certain transform applied.

So yeah... i dont know. I kind of feel it's slowly time to have an in-browser require function to download and cache modules directly into the browser and eventually applying transforms client side if necessary and maybe even using cool tricks to share modules from client to client p2p style.

Can't wait for that to happen and then i might slowly leave the terminal and live only inside browser environments and work on bringing browser environments closer to the metal (thus - a minimal kernel with just some kind of browser environment on top) ..so all apps and all everything lives inside a browser like environment. ....boot from html file :P

@derhuerst
Copy link

I also consider it a build step.
And in theory it should be listed under devDependencies, but in practice this is currently very inconvenient and i run into missing dependencies all the time.

Also - i feel it's wrong to apply "transforms" to every dependency in the dependency tree, which seems to make it slower then necessary and introduces unnecessary risk of breaking things, by applying transforms to stuff that was never meant to have a certain transform applied.

What I meant with "build step" is that npm packages should be published in a transpired form, not transpiled in a build of the consuming package.

@serapath
Copy link
Contributor Author

i'm not sure. i don't agree with that. For which platform would you build?

The newest browsers support many features, so should they use. So should they use the built form which turns ES6 into ES5 and add some other changes even though they support ES6?

What about minification? During dev i would like to be able to step through dependency code if necessary and only minify it when i need it.

So you say you would set the package.json#main to dist/bundle.min.js or something?

@derhuerst
Copy link

In my opinion, it makes sense to have a src, lib and a dist dir in every published npm package that contains any ES6+ code. The main field would link to the ES5 lib code. browser would most likely link to the bundle in dist. jsnext:main would link to the untranspiled src dir. This way, for most of the consuming users it should just work.

If you want to generate additional ES6 bundles for modern browsers (you should generate ES5 bundles anyway to not lock people out), you can do so by configuring your bundler.

What about minification? During dev i would like to be able to step through dependency code if necessary and only minify it when i need it.

That is a different question. During dev, using a modern browser, you wouldn't bundle the code from dist, but from src (maybe only slightly transpiled, e.g. from ES7 to ES6), without minifying it.

@derhuerst
Copy link

@donmccurdy
Copy link
Contributor

Latest release (0.6.5) is now on npm. Let us know if there are any issues with this, thanks!

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

5 participants