Errors building with browserify / watchify (0.3.4)

[JedWatson]

I'm getting the following errors when building with gulp and browserify / watchify:

[13:32:43] Browserify Error [Error: Cannot find module 'browserify-shim' from 'project/node_modules/react-nested-router']
[13:32:43] Browserify Error [Error: Cannot find module 'envify' from 'project/node_modules/react-nested-router']

(clipped project path for brevity)

I've got both react and react-nested-router declared in my main package.json file:

    "react": "^0.11.0",
    "react-nested-router": "^0.3.4",

The issue seems to be that npm install doesn't install the devDependencies from modules, only the main package.json.

To resolve it I can either (a) run node install from my project's /node_modules/react-nested-router folder, or (b) declare envify and browserify-shim as devDependencies in my main package.json (this creates a version conflict though as react wants "envify": "^2.0.0" and the router wants "envify": "~1.2.0")

React declares envify as a normal (not dev) dependency:

  "dependencies": {
    "envify": "^2.0.0"
  }

... which works fine, perhaps react-nested-router could do the same with envify and browserify-shim?

[mjackson]

Hmm, yeah. I think you may be right. When you do add them as dependencies does everything work as you'd expect?

[JedWatson]

It works when I add them to dependencies to my project, I expect it'll work if they're dependencies for react-nested-router but it's hard to say 100% without having that pushed to npm.

If I hack them into my ./node_modules/react-nested-router/package.json running npm install doesn't notice any difference because it thinks everything is installed already.

[ryanflorence]

I don't get it, where in our code are we requiring browserify-shim?

[mjackson]

browserify-shim works by reading the browserify-shim section of package.json. So even though we're not explicitly requireing it anywhere in the code, it should probably still be considered a dependency, especially when people are using browserify.

[JedWatson]

It's in package.json

  "browserify": {
    "transform": [
      "browserify-shim",
      "envify"
    ]
  },

[ryanflorence]

Ah yes, I just figured that out. That's dumb, these are simply for us to create a build for bower, not to screw up your build.

1. We add this junk as dependencies
2. We make a shim/ repo instead for our bower build.

@mjackson any opinions? I'd script 2 so we'd still just do script/release, no extra effort.

[ryanflorence]

(btw, webpack works fine, ofc, because it doesn't think it needs to use the shim)

[JedWatson]

Seems like it's a good idea to add envify as a dependency.

Unless I'm mistaken, envify would be just as useful for custom browserify builds, wouldn't it? Means anyone managing their own browserify process doesn't need to know to include it as a filter. I assume this is why React has it as a dependency and a transform, but I'm a bit new to both.

(on a related note, seems like it would be safe to upgrade to envify 2.0.1, the only breaking change was supporting getter properties in custom environment objects - see hughsk/envify@624b03c)

browserify-shim on the other hand... does that add overhead if it's included as a transform when it's actually being built by browserify?

[ryanflorence]

browserify-shim on the other hand... does that add overhead if it's included as a transform when it's actually being built by browserify?

I think it just does this everywhere we require react:

var React = (typeof window !== "undefined" ? window.React : typeof global !== "undefined" ? global.React : null);

Which is not enough to worry about, but maybe that's terrible because we aren't getting the normal react from node_modules :\

[sophiebits]

I think we can specify the transforms in the build script instead of package.json so that they're used only when making the router browser builds.

[ryanflorence]

Yeah, that's what I hope to do tomorrow morning.

Sent from my iPhone

On Jul 23, 2014, at 12:23 AM, Ben Alpert notifications@github.com wrote:

I think we can specify the transforms in the build script instead of package.json so that they're used only when making the router browser builds .

—
Reply to this email directly or view it on GitHub.

[mjackson]

Okay, so I spent some time this morning figuring this out.

For building it will be fine to move the transforms from package.json to the build script. The result is the same.

Where it differs is when you're using browserify to create a build of your own project that requires the router. In that case, the browserify.transforms field in package.json tells browserify which transforms to use. So in my case, I have a project that I'm building with browserify. If I remove the transforms from the router's package.json and require("react-nested-router") in my project it doesn't pick up the shim and includes all of React.

Seems like when you're using browserify it's all or nothing.

[th0r]

@mjackson What is your problem? Does browserify include React into your bundle twice?

[mjackson]

@th0r No, it only includes React once. But I want to use CDN-hosted React.

[mjackson]

For reference, here's a branch that removes browserify.transform from package.json.

[th0r]

@mjackson So, why can't you just add browserify-shim into your app's package.json and tell to use global React variable?

"browserify-shim": {
    "react": "React"
}

[mjackson]

@th0r I am. Trouble is I need to add "react-nested-router": "ReactRouter" as well. I'm a little new to using Browserify, so I didn't understand some things. If I'm using React's global build, I need to use global ReactRouter as well.

Anyway, I think I've figured it out now thanks to @rpflorence. Let's go ahead and move the transforms to the build script.

[JedWatson]

@mjackson alternatively you should be able to exclude React from your build by following the instructions here: https://github.com/substack/node-browserify#multiple-bundles

@rpflorence does ReactRouter actually have any code branches that are excluded by envify for production builds, like React does? I did a quick search and couldn't see any references to process.env. If yes, let's keep it, but otherwise couldn't we drop it from the transforms list altogether? As I understand it, because React specifies it, ReactRouter doesn't need to.

[mjackson]

@JedWatson The react/lib code we depend on does use process.env, so I think we still need that transform.

[JedWatson]

@mjackson aaah right, of course.

Thanks for fixing this up :)