Forbid running any scripts when webpack/jest/babel are installed directly

[gaearon]

We get too many issues with people trying to update these tools manually and completely breaking setups. We should just check for those dependencies and immediately crash with an explanation that it's not supported to declare them explicitly without ejecting.

[gaearon]

Alternatively we should probably be more resilient in how we resolve dependencies so that installing incompatible versions can't break us. I'll need to think more about this.

[viankakrisna]

how about packaging react-scripts using https://github.com/zeit/pkg? will it solve the problem?

[gaearon]

Potentially, and maybe help performance too. I’m interested in seeing a proof of concept.

[viankakrisna]

I've experimented with it before, but couldn't make it work... IIRC something to do with dynamic requires. Will bundling react-scripts into one file using webpack / rollup achieve similar results?

[viankakrisna]

tried using webpack, it also complains about dynamic requires require(variable) 😄 how does one compile nodejs app to one file with all this dynamic requires? It would be tiresome to track all this in a config like https://github.com/zeit/pkg#config. (Especially if we want our deps to be included in the final bundle as well)

[wtgtybhertgeghgtwtg]

Wouldn't that make it really impractical to work with create-react-app projects in monorepos and bring up some code duplication issues if an application uses libraries that react-scripts does?

[viankakrisna]

can you share more about the monorepos use case? I think the duplication does not matter because react-scripts should be self-contained and free of outside interference. Right now you can even modify webpack.config.dev.js from the outside and call scripts/build.js. This is what i did in my project:

// build.js, call node build.js instead of react-scripts build
process.env.NODE_ENV = 'production';
const config = require('react-scripts/config/webpack.config.prod');
const RollbarSourceMapPlugin = require('rollbar-sourcemap-webpack-plugin');
config.plugins.push(
  new RollbarSourceMapPlugin({
    accessToken: process.env.REACT_APP_ROLLBAR_SOURCEMAP_ID,
    version: process.env.REACT_APP_GIT_SHA,
    publicPath: config.output.publicPath,
  }),
);
require('react-scripts/scripts/build');

This breaks the guarantee of upgradable react-scripts but a convenient way to have an "almost standard" setup. Packaging react-scripts would break this use case and even react-app-rewired, but make it more resilient to outside interference.

So yeah, packaging would be an extreme decision to make. 😄 Is there any other way to make the react-scripts always work in the jungle of different environments and interference?

[wtgtybhertgeghgtwtg]

monorepos use case

Mostly it's just having the client and server in the same place so I can work on them both or share packages between the two. It's also useful in libraries to have an examples directory, but that's another thing.

I think duplication does matter, but I'm a bit of a fanatic when it comes to deduplication, though, so take what I have to say on the matter with a grain of salt.

As far as modifying the internals, couldn't you just do what they did with react and make react-scripts a flat bundle? It'd probably be a bit of work, but no more than packaging it with pkg.

[viankakrisna]

I think duplication does matter, but I'm a bit of a fanatic when it comes to deduplication, though, so take what I have to say on the matter with a grain of salt.

Yes! I admire your work on deduplicating dependencies :)

The problem is it's quite common for server-side code to use dynamic requires, they were expected to run in node/commonjs environment which allows variable to be used as the argument for require. It's sad that it makes it harder for the consuming project to bundle their code because it can't be statically analyzed to be bundled together.

[viankakrisna]

After playing around with pkg a bit more I can see that it turns out to be easier to config the file glob for dynamic requires! You just need

// package.json
"pkg": {
    "scripts": "*.js"
},

But it chokes on non-standard js file with .js extension: 😢

Error! Unexpected token (13:7)
  /Users/adeviankakrisnafadlil/Projects/create-react-app/packages/react-scripts/node_modules/ast-types-flow/lib/types.js

[wtgtybhertgeghgtwtg]

I was thinking about this today. What would the story be about ejecting with this?

[viankakrisna]

@wtgtybhertgeghgtwtg I think we can just copy / create the the source to the user folder? Opened #3704 for the POC work on this

[gaearon]

The problem with bundling is it will break semi-supported use cases like react-app-rewired. We’re not really supporting them but IMO we shouldn’t break them without a big benefit either. Maybe bundling everything except config folder would be a good compromise.

[gaearon]

For this problem I think the more immediate solution is to figure out what breaks when user installs Jest/Babel/ESLint/Webpack themselves and fix it. Ideally we should just ignore them.

[omeid]

I think the most straight forward solution is the best, check if webpack/jest/babel are installed directly, or unsupported versions are being pulled because of other dependencies.

An engineering solution to this is bound to bring up it's own complexity and problems, is it worth it?

[omeid]

And with babel-plugin-macros on the way, there will be no need for react-app-rewired (at least in majority of cases) and other shenanigans. I would really like to see a simple and practical solution instead of trying to work around or over-engineer things.

[gaearon]

We could check but it seems like the root of the issue is that we incorrectly resolve dependencies somewhere. Just installing something at the project root shouldn’t break the build.

[GAumala]

Sure, it should be unacceptable to break your build just because you wanted to install something like eslint at project root, but what can CRA do about it? Shouldn't that be an issue with the package manager? react-scripts already specifies its dependencies as precisely as it can, doesn't it?

[jlongster]

Here's a use case and I'm not sure how it would fit in here:

• I have a monorepo with a package created by create-react-app. It is a "design" package that contains a bunch of core components that implement a design for the app. I can go in there and run the project and get a running live style guide in the browser that shows off these components, with hot reloading to develop them.

• My main app is another package that consume this repo. To make this repo consumable by another repo, since tools don't transform stuff from node_modules and this is crossing that boundary, I have to manually run babel on the design package myself into a lib directory to be consumable to the rest of the app.

I guess I don't care about installing babel myself at all, but I need to be able to run babel directly.

I'm also not sure jest should be included here. Using the testing stuff from CRA is optional and if you wanted to you should be able to use an entirely different version of jest and do testing yourself, you should be able to. At least that's what I do (not sure if it's a different version, but I just use jest directly).

[gaearon]

react-scripts already specifies its dependencies as precisely as it can, doesn't it?

Again, that is exactly my point. Since react-scripts specifies its own dependencies, installing something at top level shouldn't possibly break it. Since it does (based on reports) there's a bug in how we resolve dependencies somewhere.

[gaearon]

Just realized we already have an issue tracking this.
#1795

[gaearon]

Fixed by #3771.