thlorenz / browserify-shim

📩 Makes CommonJS incompatible files browserifyable.
MIT License
933 stars 87 forks source link

Remove browserify as a peerDependency #184

Closed sterpe closed 9 years ago

sterpe commented 9 years ago

This is related, of course to #7.

The peerDependency makes it very unwieldy to have an ecosystem of modules that declare their local transforms in the browserify transform field of the package.json and provide those transforms as deps as described in the browserify-handbook. When the transform is browserify-shim we end up with a bazooka install of browserify + the need to compile bindings for contextify, for every package that needs the shim. Even worse if you are serving these packages out of github repos where module deduping doesn't seem to work as nicely.

Other points are that auto-install of peerDependencies is going away anyway and the fact that there are probably very few if any browserify < ~2 out in the wild anymore, which was largely the point of #7.

@domenic, thoughts?

bendrucker commented 9 years ago

Can you be a bit more specific about the problems you're running into with peer deps?

sterpe commented 9 years ago
npm install
npm WARN peerDependencies The peer dependency browserify@>= 2.3.0 < 12 included from browserify-shim will no
npm WARN peerDependencies longer be automatically installed to fulfill the peerDependency 
npm WARN peerDependencies in npm 3+. Your application will need to depend on it explicitly.
@foo/window@0.0.0 node_modules/@foo/window
├── browserify@11.0.1 (https-browserify@0.0.0, htmlescape@1.1.0, tty-browserify@0.0.0, isarray@0.0.1, inherits@2.0.1, builtins@0.0.7, string_decoder@0.10.31, constants-browserify@0.0.1, path-browserify@0.0.0, os-browserify@0.1.2, browser-resolve@1.9.1, commondir@0.0.1, defined@1.0.0, process@0.11.1, stream-browserify@2.0.0, xtend@4.0.0, shell-quote@0.0.1, punycode@1.3.2, domain-browser@1.1.4, querystring-es3@0.2.1, assert@1.3.0, timers-browserify@1.4.1, deps-sort@1.3.9, util@0.10.3, events@1.0.2, parents@1.0.1, has@1.0.1, vm-browserify@0.0.4, console-browserify@1.1.0, subarg@1.0.0, url@0.10.3, shasum@1.0.1, through2@1.1.1, readable-stream@2.0.2, duplexer2@0.0.2, concat-stream@1.4.10, stream-http@1.4.1, read-only-stream@1.1.1, buffer@3.4.1, resolve@1.1.6, glob@4.5.3, labeled-stream-splicer@1.0.2, browser-pack@5.0.1, JSONStream@1.0.4, syntax-error@1.1.4, crypto-browserify@3.9.14, insert-module-globals@6.5.2, browserify-zlib@0.1.4, module-deps@3.9.0)
└── browserify-shim@3.8.10 (through@2.3.8, resolve@0.6.3, mothership@0.2.0, exposify@0.4.3, rename-function-calls@0.1.1)

@foo/document@0.0.0 node_modules/@foo/document
└── @foo/window@0.0.0 (browserify@11.0.1, browserify-shim@3.8.10)

It just feels like that is a bazooka, a bit. But it can be worse if modules in the ecosystem are using a mixed bag of browserify-shim versions, that might otherwise be fine with the actual top-level version of browserify running the compile but which to my understanding will force the installation of a bunch of different versions of browserify pointlessly at the local dependency level.

Just killing a of lot trees for very little added value, given that peerDependencies are going away.

domenic commented 9 years ago

peerDependencies are not going away. They are becoming a mechanism to give appropriate warnings, which is super-useful. This should definitely stay.

sterpe commented 9 years ago

So maybe I'm misunderstanding how this works? In the case above, I would only end up compiling bindings for browserify 1x -- then after that is it just sitting in the module cache?

domenic commented 9 years ago

I don't really understand what "compiling bindings" means. You also reference contextify which is a bit bizarre since neither browserify-shim nor browserify has anything to do with contextify.

sterpe commented 9 years ago

Ok. I just realized I'm a moron. I have jest-cli sitting along side it....and that's what's so heavy.

I'll close this issue.

Thanks guys.