REMOVE nodeenv development

[ndelangen]

fixes: #68

[tmeasday]

What about the BROWSER bit? I don't know why that's there either.

[tmeasday]

The commit where it came from is super old: https://github.com/chromaui/chromatic/commit/e1a9ba09283ff96a6c9b369e855db5c3bd41efba

[ndelangen]

The NODE_ENV=TEST was to make babel parse the example storybook correctly 😖

Removing it caused the example to fail on require.context

I was able to make the example work by removing the babel stuff for webpack's require.context, and migrating to tri-config.

[ndelangen]

I merged in the better error handling, because it actually helped my find & test the problem.

I could remove it from this branch again if you really want @tmeasday or I could close the other PR, and consider it 1 cleanup PR. WDYT?

[tmeasday]

Hold on a sec, you might need to talk me through what happened here.

Definitely pull the other PR out as I had comments on that separately.

[ndelangen]

I cleaned the PR up @tmeasday

[tmeasday]

Thanks it looks reasonable. I'm just worried that you had to do this. Does it mean that Chromatic doesn't work any more with non-main.js storybooks?

[ndelangen]

NO AFAICS it should work without / should work with the old config.js just fine @tmeasday

[tmeasday]

So why did you need to do it in our test storybook? Can you explain what happened? Is the BROWSER var needed?

[ndelangen]

@tmeasday build-storybook uses the babel config defined in the root. It's always done that, but because NODE_END was set to 'development' and in the root there's no babel config for that env, it wouldn't override anything.

Removing NODE_ENV='development' made it act as NODE_ENV='test' which DOES match a env specified in the root babel config, and thus it's applied.

Turns out if we add NODE_ENV=test here:

chromatic-cli/bin/register.js

Line 3 in fb07c93

process.env.NODE_ENV = process.env.NODE_ENV || 'test';

IMHO that should be production WDYT?

[ndelangen]

We can't know exactly what impact a NODE_ENV change will have on the users config/code-output. Perhaps it's safer to release this as a major?

[ndelangen]

AFACS there's no usage of process.env.BROWSER whatsoever. Removing seems to do absolutely nothing.

[tmeasday]

OK, setting NODE_ENV=test there seems not correct either. Do you know why we did that? Was it for the sake of dotenv?

We can't know exactly what impact a NODE_ENV change will have on the users config/code-output. Perhaps it's safer to release this as a major?

You are probably right.

[ndelangen]

I'll delete this code and that makes this a major release

process.env.NODE_ENV = process.env.NODE_ENV || 'test';