Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Properly set mode and NODE_ENV for test middleware #900

Closed
eliperelman opened this issue May 23, 2018 · 3 comments
Closed

Properly set mode and NODE_ENV for test middleware #900

eliperelman opened this issue May 23, 2018 · 3 comments
Assignees
Labels
Milestone

Comments

@eliperelman
Copy link
Member

In the v8 line of Neutrino we manually default NODE_ENV to test when running the test command. With the removal of the Neutrino CLI, we would no longer have a test command, but we do still have testing middleware via the karma, mocha, and jest presets. We should ensure that we have a reasonable value for the config.mode and NODE_ENV for test middleware to rely on while trying to get the least number of config permutations.

At the outset, I'm wondering if we should have --mode none and NODE_ENV=test, but I think @edmorley had concerns about this particular mode. Also need to ensure that these values can be easily overridden from the host environment.

@eliperelman eliperelman added this to the v9 milestone May 29, 2018
@edmorley
Copy link
Member

edmorley commented Jun 1, 2018

I've been experimenting with setting mode to development when using Karma.

I've finally got it working, however I can see from the webpack build console output that the hot reloader configs are being added. This doesn't break anything, but is another side-effect of consolidating 9+ different permutations of configuration (command = {start, build, test, ...} x NODE_ENV = {production, development, test}) into just two (mode = {development, production}).

I'd be open to considering mode=none as a way to add a third state, though it would disable some of the webpack development features that might be useful for testing (eg named modules), so we'd want to vet those:
https://github.com/webpack/webpack/blob/v4.10.2/lib/WebpackOptionsDefaulter.js#L257-L266

Other possible options:

  1. Make Neutrino support a mode of 'test', that gets translated to 'development' in the webpack config, but sets NODE_ENV to 'test'. Presets would then be encouraged to check the value of NODE_ENV rather than mode, so they would see all three states, not just the 2 stats of mode. (Though having Neutrino's mode support extra options is kinda confusing, so not sure this is ideal.)

  2. Encourage presets to use NODE_ENV to check for development/production/test, and allow the adapters (neutrino().karma() etc) to set both a default mode and NODE_ENV.

@edmorley
Copy link
Member

edmorley commented Jun 8, 2018

This is a blocker for releasing a v9 alpha. @eliperelman do you know when you might be able to take a look at this?

@edmorley edmorley added the bug label Jun 8, 2018
@eliperelman
Copy link
Member Author

I'll try and start on this today.

@eliperelman eliperelman self-assigned this Jun 13, 2018
edmorley added a commit that referenced this issue Jul 13, 2018
Previously the only way to override `mode` was by passing `--mode` on
the command line. However this is not possible with all tools, since
some (such as karma) reject unrecognised arguments.

Now:
* `mode` is derived from `NODE_ENV` if `--mode` wasn't passed, and
  !production `NODE_ENV` falls back to mode `development`.
* if `--mode` is passed, it takes priority over `NODE_ENV`.
* if neither `mode` nor `NODE_ENV is defined, then `NODE_ENV` is set
  to `production`, however `mode` is left undefined, which causes
  webpack to output a helpful warning about relying on defaults.
* the template test runner configs set a default `NODE_ENV` of `test`.
* `@neutrinojs/stylelint` now also correctly sets `failOnError`.

Fixes #900.
Fixes #971.
Closes #955.
@edmorley edmorley assigned edmorley and unassigned eliperelman Jul 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants