browserify / wzrd.in

browserify as a service.
http://wzrd.in
MIT License
637 stars 96 forks source link

Transforms works when included in dependencies, but not in devDependencies or otherDependencies #166

Open snajjar opened 7 years ago

snajjar commented 7 years ago

Hello there,

Thanks a lot for this awesome projects. I've been scratching my head around this: It seems that all browserify transforms (babelify, cssy, rollupify, etc.) are found by browserify in the module compilation when included in package.json dependencies, but not in devDependencies or in otherDependencies. Is there a way to make wzrd.in works when transforms are declared as such in package.json?

jfhbrook commented 7 years ago

It does a production install https://github.com/jfhbrook/wzrd.in/blob/master/bundler/install.js#L12 meaning no devDeps. Without changing this policy, there's no way around this.

snajjar commented 7 years ago

But when including it as a dependency, if i'm not wrong, the transforms code will be included in the bundle, increasing the bundle size, am I right? What's the reason for the production install policy? Was there a discussion somewhere about this?

jfhbrook commented 7 years ago

The transforms code won't be included in the browserify bundle unless the browserified module actually requires it.

jfhbrook commented 7 years ago

As far as why? The idea is that you shouldn't need, say, a test harness, to browserify a module. This decision was made prior to any support for transforms that may or may not be included in wzrd.in (I don't remember any transforms support but I could be wrong on that)

snajjar commented 7 years ago

Makes sense, thx for the explanations. If i submit a PR to make the --production flag actionable from command line, would it be integrated? Or should I just manage to run my own fork? I'm fine with both.

jfhbrook commented 7 years ago

you could just include transforms in dependencies...

...but no, not at this time. There's a major refactor or two that I'd like to merge and deploy before making any other changes.

jfhbrook commented 7 years ago

So I had a bit of a realization on my way home. I don't plan on building this soon but capturing this use case.

This may or may not only make sense if transform flags can be passed to the builder.

The idea is for multibundles to install deps that are required for building the project, but not required for being included in the returned bundle. One way of doing this could be to install but not bundle devdeps. There are likely other ways too besides overloading that particular concept.

snajjar commented 7 years ago

Yes, it only need to be installed, not bundled (that's why I didn't understand the policy at first, but sure, it can speed up wzrd.in if you don't install all the testing and linting stuff).

Maybe an alternative way is to install devDeps only if in package.json, "browserify" field is declared? That would support transforms, but remain as quick as possible for the modules that doesn't need transforms.

jfhbrook commented 7 years ago

That would support transforms

So I'm a little confused because afaik wzrd.in doesn't support transforms. Could be it does and I'm forgetting, but I can't find it in the source code! This ticket would only apply if we were doing that (and I don't think we are).

snajjar commented 7 years ago

Hahaha, this comment just made my day!

Actually, wzrd.in can support browserify transform in a module with the when the transform is declared in the package.json: 1) include the transform package in dependencies (say "babelify") 2) include the transform definition in the package.json.

I made it work for some package.json as such:

{
  ...
  dependencies: {
    ...
    "babelify" : "*",
    "babel-preset-es2015" : "*"
  },
  "browserify": {
    "transform": [["babelify", { "presets": ["es2015"] }]]
  }
}

I've also made it work with other transforms, like cssy or rollupify. You don't need any special code for it, just browserify -r will work when the package.json contains the transform definition.