bendrucker / ama

Ask me questions about building web applications
MIT License
6 stars 1 forks source link

Writing a library with browserify #7

Closed thaiat closed 9 years ago

thaiat commented 9 years ago

Hi @bendrucker,

This post is a continuation of our discussion here: https://github.com/substack/node-browserify/issues/1102

So i published to npm this repo: https://github.com/Yoobic/yoobic-angular-core

When i require('yoobic-angular-core') in a new project (let's say app-directive) everything works fine. Then, if i change the browser-shim entry of package.json in yoobic-angular-core from "global:angular" to "angular", i get the following error when trying to bundle the consumer package (app-directive).

Browserify failed
 ENOENT, open '/Users/thaiat/Documents/Dev/app-directive/node_modules/yoobic-angular-core/bower_components/angular/angular.js'

The package.json of the consumer project properly defines a browser-shim for angular. I'm trying to figure out how to make browserify understand it should uses the location of angular coming from the consumer package.

A similar use case, is that my library package uses let's say angular-famous. So in it's code i will do require('famous-angular') with the proper browserify-shim defined in its package.json Then on the consumer side, i want also to require ('famous-angular'), and i don't want to end up with 2 copies of 'famous-angular'. Also i don't think famous-angular can be defined as global.

Le me know what you think.

bendrucker commented 9 years ago

Browserify will automatically dedupe for you. You should try that and see if that's working as expected.

But you need to be installing Angular from npm. You have to go full npm or you're just creating new headaches. Using b-shim to change Angular into a global isn't really the correct solution, but definitely by far the easiest. As you mentioned, it doesn't work in all cases and the correct way to handle things is really to let browserify de-dupe.

That puts the onus on the package maintainer (i.e. you) to be smart about semver. If your package is compatible with Angular 1.3, you need "angular": "~1.3.0". Browserify only dedupes precisely identical code for obvious reasons and so you have to make sure that your users will end up with an identical Angular version for this whole thing to work. As long as you're running a fresh npm install before you build (hopefully you're doing CI anyway so that's a given), you should get identical versions in the node_modules hierarchy that can then be de-duped.

Also, there are still some outstanding issues around the tooling that need fixing, specifically around browserify-shim. One is being able to shim packages by name rather than path (https://github.com/thlorenz/browserify-shim/issues/107) which will be particularly useful for all the grossly misbehaved Angular modules out there. There might be a few others. At some point I'm going to switch from script concatenation for my vendor bundles and have to wrestle with all the bugs and edge cases and they'll get fixed.

thaiat commented 9 years ago

i would love to use only npm, but not every client library exists on npm with the latest version. This is the case for famous-angular which npm version is not up to date.

i have found a hacky workeround though. In my library package, for every require of angular modules that would not exist on npm ( famous-angular, etc...) and that need to be provided by the consumer, i use the following shim "global:null"

Now i also tried another approach that didnt work: I bundle the library package with browerify, excluding every require that is not my own code The consumer require this bundled library. But it looks like requiring a browserified code does not work.

bendrucker commented 9 years ago

If you want to use bower deps, you need to commit them or have bower as a dep and then use a postinstall npm hook.

Your workaround is unnecessary. Use Angular's DI system and don't require something if you're expecting it to already exist and not expose a proper CommonJS module.

thaiat commented 9 years ago

agreed, the workaround is there only for allowing to test properly my library during CI.

Let say i want to build a library of famous angular directive, i need famous-angular to be available when i unit test the directive, otherwise i cannot check properly the resulting html.

I could also include famous angular in karam config, but it's simplier if karma does not have to know about the dependencies.

You can have a look at the latest commit in https://github.com/Yoobic/yoobic-angular-core.

thaiat commented 9 years ago

also i can easily bower install on the npm postinstall hook of the library, but then i end up with 2 angular in the consumer project...

bendrucker commented 9 years ago

Gotta point me to something specific if you want me to look. Still not understanding what the problem here is.

thaiat commented 9 years ago

ok here are the steps to reproduce the various issues:

ATTEMPT 1:

ATTEMPT 2:

ATTEMPT 3:

bendrucker commented 9 years ago

Re: 3, are you bundling the same angular file? It's possible the release on bower is slightly different than npm and so browserify can't dedupe.

thaiat commented 9 years ago

it's exactly the same one, same browser entry in package.json, same version in bower.json

bendrucker commented 9 years ago

If you get me something I can clone and reproduce I'll take a look.

bendrucker commented 9 years ago

Willing to spend time because I'm going to have to solve this problem myself and have just been dodging it by assuming window.angular instead (using b-shim's global). Please keep the repro simple so I can clone, install, and run.

thaiat commented 9 years ago

cool, i will prepare for you a clean and simple repo so you can reproduce on your side, thks

thaiat commented 9 years ago

here you go : https://github.com/thaiat/app-browserify

bendrucker commented 9 years ago

No time this week to look, but can take a few minutes this weekend. Ping me via this thread on Saturday.

thaiat commented 9 years ago

ok I will

thaiat commented 9 years ago

ping

bendrucker commented 9 years ago

This suggest that browserify is looking for angular inside the submodule instead of de-duping from the parent module location

Expected. Browserify still has to be able to find the file to know it's a duplicate. Can't just happen by using the same name.

When everything's installed properly, sounds like you're running into https://github.com/thlorenz/browserify-shim/issues/132

This is just not going to work well until https://github.com/angular/angular.js/pull/10732 lands and you can avoid shimming Angular entirely. For now just have your child packages expect window.angular. Even if you shim it'll still be exposed and even when that PR lands it'll still be present for compatibility with all the many packages expecting it.