Make dependencies of react package optional

[qerub]

Rationale: The package is entirely usable without the envify and fbjs dependencies.

This change might affect projects using npm install --no-optional.

[qerub]

CLA Signed BTW.

[quantizor]

Why not devDependencies then if they aren't needed?

[gaearon]

I might be wrong but I think fbjs is used internally in React. That’s why it was created: as a way for Facebook projects to share some common code. It is an implementation detail and should not affect you externally.

[zpao]

Fbjs is not optional. Envify is there for use with browserify so could probably be called optional.

[gaearon]

I don’t think making envify an optional dependency makes sense either. Browserify build will fail if a package specifies a transform that doesn’t happen to be installed. Doesn’t seem like there is ever a case when we want react to get installed without envify coming with it. Of course it only matters to Browserify users, but we can’t guess the bundler a person is using.

[qerub]

(First of all, thanks for the involvement and sorry for creating a pull request instead of an issue even though I knew this wouldn't be a drive-by pull request.)

I'm claiming the package is usable without the fbjs dependency because the distribution files (in dist/) have a bundled/inlined copy of the library. It seems a bit strange to me to bundle one copy and then force installation of a second copy through the dependency list.

Regarding envify, would it be possible to split out Browserify-specific functionality into another package (e.g. react-browserify)? I'm not using Browserify and am not too happy with having envify and it's transitive dependencies {jstransform, base62, esprima-fb, source-map, amdefine, through} lying around doing nothing but complicating future application maintenance (think patch/vulnerability management).

[gaearon]

I'm claiming the package is usable without the fbjs dependency because the distribution files (in dist/) have a bundled/inlined copy of the library. It seems a bit strange to me to bundle one copy and then force installation of a second copy through the dependency list.

Yeah, but the same can be said about React itself. The fact that it exists in dist doesn’t mean we should get rid of lib folder. The dist files exist for consumers who want to use UMD builds via npm. For example, this is useful when you swap CommonJS modules for UMD builds in production configuration. However, the main contents of the npm packages are still CommonJS modules, and those won’t work without fbjs being a dependency (also consumed via CommonJS).

Regarding envify, would it be possible to split out Browserify-specific functionality into another package (e.g. react-browserify)? I'm not using Browserify and am not too happy with having envify and it's transitive dependencies {jstransform, base62, esprima-fb, source-map, amdefine, through} lying around doing nothing but complicating future application maintenance

React relies on envification in order for CommonJS consumers to be able to get both development and production build depending on the configuration. Different build tools have different approaches to envification. Webpack has the stance that libraries should ship compiled code, and any further transformations should be done manually by the user if necessary. Browserify decided that libraries can declare the necessary transformations. Both approaches have upsides and downsides, but they are already ingrained in the respective communities.

If we remove the Browserify transformation step, most today’s Browserify consumers will get development builds in production. This is because, without envify, Browserify will still provide a polyfill for process.env, but NODE_ENV will be undefined. This means that introducing such a change will have a long-lasting impact of forcing pretty much every Browserify user change their React build setup to envify their build globally just because React decided to change their mind. Even if you consider Browserify design suboptimal, you can probably agree this would be a very devastating change especially considering that many people are going to miss it, and as a result will end up with slower apps running developer builds in production.

Having a separate package like react-browserify could be an option, but it is going to cause a ton of confusion. What about related packages like addons? What about code examples that all use require('react') or import React from 'react'? It would be very unfortunate to ask people to pick different NPM packages based on their bundler, and some people inevitably will use the wrong one, so this doesn’t seem like a viable option.

Browserify was the first successful mainstream CommonJS bundler and we can’t ignore its conventions even if in some cases they seem suboptimal. Having transitive dependencies because of a build tool you don’t use can indeed be frustrating, but breaking all users of the most popular bundler or introducing major inconsistency in using React with it seems like a worse kind of problem to me. I think that with the current situation your best bet would be to either use React from a CDN, or set up a mirror npm package where you only bring the dist files and remove the dependencies. Sorry I can’t be more helpful than that!

[zpao]

Great back and forth, thanks for explaining it Dan. Apologies for my curtness, I'm on my phone at the beach 🏖

One option to cut down on deps for the browserify case is to switch to using loose-envify which only has 1 additional dependency and should work fine since we don't have any crazy uses of process.env.

[gaearon]

switch to using loose-envify which only has 1 additional dependency and should work fine since we don't have any crazy uses of process.env

Sounds great. @qerub Would you like to take a stab at PR?

[gaearon]

I am closing this per the discussion above.

We would be happy to accept a different PR that switches React to use loose-envify instead of envify. As noted above this should get rid of most of these transitive dependencies.

Thank you for taking time to contribute, and we hope to see you again!

[qerub]

Thanks for the thorough explanation of your reasoning. I have now submitted a pull request (#6303) for the loose-envify suggestion.