neutrinojs / neutrino

Create and build modern JavaScript projects with zero initial configuration.
https://neutrinojs.org
Mozilla Public License 2.0
3.94k stars 213 forks source link

Make optional to override NODE_ENV #1231

Closed yordis closed 5 years ago

yordis commented 5 years ago

Hey folks, tool like this one https://github.com/facebook/create-react-app/blob/5352a0c0467dcf3e873dfc7b93930515c373149d/packages/babel-preset-react-app/dependencies.js#L34 they are doing micro-optimizations like https://github.com/facebook/create-react-app/blob/5352a0c0467dcf3e873dfc7b93930515c373149d/packages/babel-preset-react-app/dependencies.js#L70

Since you are controlling the value of NODE_ENV here https://github.com/neutrinojs/neutrino/blob/b6372ab2539a3551eefb3c660735a12875e10f4e/packages/neutrino/index.js#L14-L25 and you are overwritting it with development. Now I am having a hard time adding that preset.

Unless there is an strong tecnical reason for doing this, I would prefer that Neutrino do not get in my way when it comes to take these decisions, how I manage my environment should me my responsability.

I use Neutrino for the arcthiecture of middlewares and the amazing API for managing the Webpack config and I would love to see the package become even simpler and moving away from these type of decisions.

Thoughts on that?

edmorley commented 5 years ago

Hi!

We're not overwriting NODE_ENV if the user specifies it explicitly; what you're asking for should already be possible? Could you explain more about the problem you are seeing?

Unless there is an strong tecnical reason for doing this, I would prefer that Neutrino do not get in my way when it comes to take these decisions, how I manage my environment should me my responsability.

There are strong technical reasons though. An unset NODE_ENV causes many problems with Babel/webpack (see the webpack issue tracker for examples). The whole point of Neutrino is to configure the environment in a way that means end-users don't encounter these issues, but also allows all configuration to be overridden.

What is the use-case for using CRA's Babel preset out of curiosity?

yordis commented 5 years ago

There are strong technical reasons though. An unset NODE_ENV causes many problems with Babel/webpack (see the webpack issue tracker for examples).

I totally undertand this but that is a programmer not paying attention.

The whole point of Neutrino is to configure the environment in a way that means end-users don't encounter these issues, but also allows all configuration to be overridden.

Totally.

We're not overwriting NODE_ENV if the user specifies it explicitly; what you're asking for should already be possible? Could you explain more about the problem you are seeing?

I am an idiot 🤷‍♂️ 😅 It is 4:27AM and I should go to sleep already since I have to wake up at 8AM ...

yordis commented 5 years ago

What is the use-case for using CRA's Babel preset out of curiosity?

That preset is micro-optimizing based on the environment and has a lot of details that we will probably miss unless we are aware of it.

edmorley commented 5 years ago

I'm guessing the issue is that --mode is being specified in addition to NODE_ENV in this case, meaning--mode "wins"?

ie:

NODE_ENV=test webpack --mode development
# Inside the build: mode=development; NODE_ENV=development

If --mode is omitted then it currently results in the desired behaviour for this case:

NODE_ENV=test webpack
# Inside the build: mode=development; NODE_ENV=test

We could instead change Neutrino to take into account both --mode and a user's NODE_ENV, which would give:

NODE_ENV=test webpack --mode development
# Inside the build: mode=development; NODE_ENV=test

However that gives a bad outcome in cases where the NODE_ENV is set to an opposite value (such as can happen in Heroku builds/CI runs, where the user might not even know that NODE_ENV has been set for them). For example:

NODE_ENV=production webpack --mode development
# Inside the build: mode=development; NODE_ENV=production

For more reasoning on the current implementation, see: https://github.com/neutrinojs/neutrino/pull/955#issuecomment-399957959 (this issue relates to "Scenario G" in that comment)

What is the use-case for using CRA's Babel preset out of curiosity?

That preset is micro-optimizing based on the environment and has a lot of details that we will probably miss unless we are aware of it.

We tend to pay close attention to what CRA does to ensure we're not missing optimizations, as such it mostly shouldn't be necessary to use their Babel preset. The things that they do, that we don't are mainly because they offer no way to modify the configuration, so have to pick something that works for everyone (such as compiling node_modules, which is an anti-pattern that should be avoided unless absolutely required) - whereas we can document how to tweak the config for those specific use-cases (example: https://master.neutrinojs.org/packages/compile-loader/#compiling-node_modules).

If you spot any features from their preset that you think should be a part of one of Neutrino's presets, or be documented - please let us know :-)

yordis commented 5 years ago

The things that they do, that we don't are mainly because they offer no way to modify the configuration,

Yeah, the reason why I love so much Neutrino and also why I start creating packages like https://github.com/straw-hat-team/jest-config where I want to have good defaults but I want the flexibility of keep modifying it. Just give me a bunch of factories.

Right now adding Neutrino to my CLI like react-script (but much more) but fully customizable thanks to Neutrino.

But to be honest, I never used a Neutrino preset before, I roll my own 😄 hold habit of Webpack veteran.

yordis commented 5 years ago

(such as compiling node_modules, which is an anti-pattern that should be avoided unless absolutely required)

Hah interesting, I actually prefer to have this. If library authors compile their packages to the latest ECMAScript would be better IMO.

Since compilers like Webpack or Babel are getting better at caching, that first run is not a bid deal but at least you get more benefits from having uncompiled source code (well at least no compiled to a version like ES5)

Tree-shaking, polyfilling and other use cases that are completely related to your environment are not a concern for library authors anymore, since you control your environment, do whatever you want 😄

As long as library author are discipline for sure.

That feature is actually really handy.