Storybook: defining `process` as `true` breaks usage of libraries looking for environment variables

Created on 10 May 2019 · 15Comments · Source: storybookjs/storybook

Describe the bug

process is defined as true using the webpack define plugin. true does not match the expectation that process is an object.

this results in libraries, even when attempting to be defensive about the presence of process, attempting to perform object actions against a boolean and therefore throwing errors at runtime.

the specific example that is giving me trouble is with the browser version of debug.

To Reproduce

Steps to reproduce the behavior:

import debug into one of your stories
navigate to that story within storybook
See error in the browser console

Expected behavior

process should either be left as undefined and expected to be added by projects that need it or at least defined using a shape that matches the expected contract. please also see this comment from a maintainer of debug about the contract of process

error

the error thrown in the case of debug

TypeError: Cannot use 'in' operator to search for 'env' in true
    at Function.load (browser.js?34eb:221)
    at setup (common.js?dc90:261)
    at eval (browser.js?34eb:250)
    at Object../node_modules/debug/src/browser.js (vendors~main.90358a9d4fdec3ed3967.bundle.js:75360)
    at __webpack_require__ (runtime~main.90358a9d4fdec3ed3967.bundle.js:782)
    at fn (runtime~main.90358a9d4fdec3ed3967.bundle.js:150)
    at eval (any.es.js?d9f8:39)
    at Module../node_modules/@travi/any/lib/any.es.js (vendors~main.90358a9d4fdec3ed3967.bundle.js:68352)
    at __webpack_require__ (runtime~main.90358a9d4fdec3ed3967.bundle.js:782)
    at fn (runtime~main.90358a9d4fdec3ed3967.bundle.js:150)

Code snippets

https://github.com/visionmedia/debug/blob/5c7c61dc0df0db4eb5de25707d8cd1b9be1add4f/src/browser.js#L216

System:

OS: MacOS
Device: Macbook Pro 2016
Browser: chrome
Framework: react
Addons: actions, info, links
Version: [e.g. 4.0.0] found initially on v5.1.0-alpha.40, but have confirmed that it still exists in v5.1.0-beta.0

Additional context

i did add a comment yesterday evening against the commit that appeared to add this definition. its it can be bad form to comment against closed issues, i figured a more formal issue could be better. please don't see the double filing as any additional pressure from me. i'm just trying to follow up to get you the info that aligns with your normal process.

babel / webpack bug high priority

Source

travi

👍4

Most helpful comment

Here's the workaround I put in place for now (requires full control mode):

module.exports = ({ config, mode }) => {
  const plugins = [
    ...config.plugins
      // remove instances of DefinePlugin to work around:
      // https://github.com/storybooks/storybook/issues/6763
      .filter((plugin) => !plugin.definitions),
    // replace the default DefinePlugin instance with a custom one that tries to
    // preserve everything except for `process: true`
    new webpack.DefinePlugin({
      "process.env": {
        NODE_ENV: JSON.stringify(mode.toLowerCase()),
        NODE_PATH: JSON.stringify(""),
        PUBLIC_URL: JSON.stringify("."),
      },
      NODE_ENV: JSON.stringify(mode.toLowerCase()),
    }),
  ];

  return {
    ...config,
    plugins,
  };
};

lewisl9029 on 10 May 2019

❤2

All 15 comments

I was just about to report the exact same issue. In my case our project used https://github.com/nygardk/react-share, which had debug as a transitive dependency.

This also appears to overwrite whatever was set previously as process.env in the previous DefinePlugin invocation above, which also feels problematic: https://github.com/storybooks/storybook/pull/6553/files#diff-87391fa0641e90443c27a7173650c0dbR58

lewisl9029 on 10 May 2019

This also appears to overwrite whatever was set previously as process.env in the previous DefinePlugin invocation above

thats a good point that i forgot to address in my initial issue. i did attempt to use the define plugin to override what was set as process by storybook, but i was unsuccessful. i'm not sure if there is a way to use the define plugin multiple times or not, but i was unsuccessful in my attempts.

travi on 10 May 2019

Here's the workaround I put in place for now (requires full control mode):

module.exports = ({ config, mode }) => {
  const plugins = [
    ...config.plugins
      // remove instances of DefinePlugin to work around:
      // https://github.com/storybooks/storybook/issues/6763
      .filter((plugin) => !plugin.definitions),
    // replace the default DefinePlugin instance with a custom one that tries to
    // preserve everything except for `process: true`
    new webpack.DefinePlugin({
      "process.env": {
        NODE_ENV: JSON.stringify(mode.toLowerCase()),
        NODE_PATH: JSON.stringify(""),
        PUBLIC_URL: JSON.stringify("."),
      },
      NODE_ENV: JSON.stringify(mode.toLowerCase()),
    }),
  ];

  return {
    ...config,
    plugins,
  };
};

lewisl9029 on 10 May 2019

❤2

@libetl Didn't you run into this bug and fix it? Can you issue a small PR for that?

shilman on 11 May 2019

Exactly this bug so I made a workaround in poc/addon-editor branch

libetl on 11 May 2019

The above snippet looks good, I also suggest to leave a value for the process.browser boolean

libetl on 11 May 2019

@libetl Can you make a PR onto next to fix the bug so we don't need a workaround at all?

shilman on 11 May 2019

Will do

libetl on 11 May 2019

pull request is now opened

libetl on 11 May 2019

❤1

since i saw that this got merged, i decided to give it a try again and found that i was still presented with the same error. it took me a bit to realize that i needed to use the debug dist-tag instead of next, but these are my results with 5.2.0-alpha.18

running with the --debug-webpack flag, i see that the "Manager webpack config" now looks like a config similar to what i would expect:

...
DefinePlugin { definitions: {} },
     DefinePlugin {
       definitions:
        { process:
           { browser: true,
             env:
              { NODE_ENV: '"development"', NODE_PATH: '""', PUBLIC_URL: '""' } },
          NODE_ENV: '"development"',
          DOCS_MODE: false } }
...

but the "Preview webpack config" (i didnt realize that two independent configs needed to be changed when i opened this issue) still shows a problematic config:

...
DefinePlugin {
       definitions:
        { 'process.env':
           { NODE_ENV: '"development"', NODE_PATH: '""', PUBLIC_URL: '"."' },
          process: 'true',
          NODE_ENV: '"development"' } },
...
DefinePlugin { definitions: {} }
...

does this mean that both need this change applied?

travi on 28 May 2019

@travi You should be able to get this on the next dist tag. The debug tag includes the stuff from the next tag, but it's main purpose is as a release channel for SB Docs, and it's not intended to be stable. Does the latest version on next (5.1.0-rc.2?) still not work for you? Do we need to apply the change to both manager & webpack configs?

shilman on 28 May 2019

Do we need to apply the change to both manager & webpack configs?

from what i can tell, the change is more necessary on the preview config than on the manager config. i was able to apply the workaround from above to get past it for now, but the change that was merged does not fully resolve the issue for me.

travi on 28 May 2019

@travi Thanks for the quick response. We'll try to get it taken care of soon.

shilman on 28 May 2019

👍1

thank you 🙏

travi on 28 May 2019

Fixed in https://github.com/storybookjs/storybook/pull/6946

Released in https://github.com/storybookjs/storybook/releases/tag/v5.1.0-rc.4

shilman on 4 Jun 2019

Was this page helpful?

0 / 5 - 0 ratings