storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.02k stars 9.23k forks source link

process.env is empty since 6.4.10 #17336

Open thany opened 2 years ago

thany commented 2 years ago

Describe the bug At runtime, inside of a story, we need access to a (subset of) process.env to pass some values from the buildserver onto Storybook, and ultimately the application proper.

In Storybook 6.4.9 this worked fine. In Storybook 6.4.10 this got broken. process.env is now { }. In Storybook 6.4.14 this is not fixed, unfortunately.

Neither env variables from the CLI are passed, nor variables from .env. We use both, and neither end up in process.env.

Is it possible that #17174 broke this? That one is the only fix that remotely seems to have anything to do with process.env, assuming the changelog is complete.

To Reproduce I'm not sure what has been done to get this to work in the first place, or whether this is standard functionality. I do see dotenv being exported from paths.js, but it's really difficult to dig into Storybook to see what it's doing with that, if anything.

System

Environment Info:

  System:
    OS: Windows 10 10.0.19043
    CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
  Binaries:
    Node: 16.13.2 - C:\Program Files\nodejs\node.EXE
    npm: 8.3.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 97.0.4692.99
    Edge: Spartan (44.19041.1266.0), Chromium (97.0.1072.62)
  npmPackages:
    @storybook/addon-a11y: 6.4.10 => 6.4.10
    @storybook/addon-actions: 6.4.10 => 6.4.10
    @storybook/addon-controls: 6.4.10 => 6.4.10
    @storybook/addon-docs: 6.4.10 => 6.4.10
    @storybook/addons: 6.4.10 => 6.4.10
    @storybook/preset-create-react-app: 3.2.0 => 3.2.0
    @storybook/react: 6.4.10 => 6.4.10
    @storybook/theming: 6.4.10 => 6.4.10
thany commented 2 years ago

As a temporary workaround, we can use this in the main.js:

module.exports = {
  webpackFinal: async config => {
    const definePlugin = config.plugins.find(
      ({ constructor }) => constructor && constructor.name === 'DefinePlugin',
    );
    if (definePlugin) {
      definePlugin.definitions = injectEnv(definePlugin.definitions);
    }

    return config;
  },
};

Where injectEnv() looks like this:

function injectEnv(definitions) {
  const env = 'process.env';

  if (!definitions[env]) {
    return {
      ...definitions,
      [env]: JSON.stringify(
        Object.fromEntries(
          Object.entries(definitions)
            .filter(([key]) => key.startsWith(env))
            .map(([key, value]) => [key.substring(env.length + 1), JSON.parse(value)]),
        ),
      ),
    };
  }
  return definitions;
}

Honestly though, I believe #17174 should be reverted, because this is quite a convoluted workaround to something that didn't need to be broken in the first place. I'm sure it wasn't intentionally broken, but it sure is a regression, iyam.

shilman commented 2 years ago

There are two conflicting scenarios: destructuring process.env and assigning to it.

6.4.9

✅  console.log(process.env.FOO);
✅  console.log(process.env);
❌  process.env.FOO = 'bar';

6.4.10

✅ console.log(process.env.FOO);
❌ console.log(process.env);
✅ process.env.FOO = 'bar';

As far as I know, there is no easy way to get all three to work. If you have a suggestion for how to revert #17174 AND not have process.env assignment break, I'm all ears.

thany commented 2 years ago

In the console, when I now log process.env, I'm seeing it as an object. There's nothing holding anyone back from adding a key to it. So, my question is, how did it break in the first place?

Question number two that mystifies me completely, process.env is an empty object. So how on great mother earth is process.env.key ever going to work? No amount of magic is going to give folks a value out of an empty object.

On top of this, when our website runs by itself, outside of Storybook, our config is set up to have process.env fully populated with the keys we need. This used to be the case in Storybook too. I cannot emphasize enough that this is a regression, and it breaks components that rely on it. I really don't think it's right to put a breaking change in a minor version update, no matter what it is. At best, this should have been kept on hold until Storybook 6.5.

chrisdrifte commented 2 years ago

It appears as though this change has caused the docs to fall out of date, as environment variables no longer work as described.

Thanks @thany for making it a quick fix.

mbjelac commented 2 years ago

spent some time reading & re-reading the docs before i found this ... could someone add a note to those docs that it actually doesn't work since 6.4.10 and link to this issue?

MatthijsBon commented 2 years ago

Is there more information about this issue yet? It is still occurring for me on storybook 6.4.19.. It would seem like a fix could be made quite easily, as the code above could be included in the default webpack config?

israelKusayev commented 2 years ago

Also accessing process.env by variable is not working which is very strange AndI it is very problematic since it works with create-react-app so our code access process.env[someEnv] in many places But it's not working when we render these components inside storybook

@shilman I hope it can be resolved soon

const name = "REACT_APP_API_URL";

console.log(process.env.REACT_APP_API_URL); // logs real value
console.log(process.env["REACT_APP_API_URL"]); // logs real value
console.log(process.env[name]); // logs undefined
console.log(process.env); // logs empty object
thany commented 2 years ago

@israelKusayev Exactly my point. What manner of black magic happens when logging some key in an object yields its real value, but the object as a whole logs empty. How? How how how?

This really feels like bad practice hackery/trickery, so I would say the cure is worse than the disease, so to speak.

MagnusHJensen commented 2 years ago

What is the status on this issue? Makes Storybook unusable, if you want some of the newer features.

ghost commented 2 years ago

Maybe the following issues are the same cause. https://github.com/storybookjs/storybook/pull/12997

ghost commented 2 years ago

Can we get an update on where this issue is at?

JasonMore commented 2 years ago

I'm in the same boat 😢

grant-davidson commented 2 years ago

It seems that in storybook 6.4.19 if your .env has:

REACT_APP_API_URL='/api'

and your code has

get(process.env.REACT_APP_API_URL)

the code actually executed is:

get('/api')

and so having code like:

process.env.REACT_APP_API_URL= '/api/test'

results in:

'/api' = '/api/test'

which doesn't end well!

It also seems that if REACT_APP_API_URL is not set in the .env then setting and using process.env.REACT_APP_API_URL in the story works as expected.

If you add

module.exports = { env: () => ({}), /* other exported properties */ }

to the default exports of main.js you'll get no imports from the .env and be able to configure them for each story as needed. This seems good to me as I would expect a story's behaviour to be predictable and independent of the local .env especially if you have tests for your stories.

gmattie commented 2 years ago

Can we get an update on this issue?!

ghost commented 2 years ago

It seems that this problem has not been solved even in Storybook ver6.5, so I will describe how to define your own environment variables in main.js when the builder is vite.

async viteFinal(config: Record<string, any>) {
    return mergeConfig(config, {
      define: {
        'process.env.STORYBOOK': true,
      },
    })
  },
tatimblin commented 1 year ago

I noticed that my .env variables for my associated application generated with create-react-app still work. So now anything I need in storybook I just prepend with REACT_APP_ even if it's not needed in the create react app application.

inxsvf commented 1 year ago

Running Storybook 7.0.12 with CRA , using a variable to read an environment value is not working.

const name = "REACT_APP_EXAMPLE";

console.log(process.env.REACT_APP_EXAMPLE);    // ✅ logs real value
console.log(process.env["REACT_APP_EXAMPLE"]); // ✅ logs real value
console.log(process.env[name]);                // ❌ logs undefined
console.log(process.env);                      // ✅ logs non-empty object
airoude commented 1 year ago

I'm having the same problem as @inxsvf; it makes storybook unusable since I'm dynamically accessing the env variables. Is there a fix or workaround?

airoude commented 1 year ago

Validating with zod also doesn't work.

import { z } from 'zod';

const envVariables = z.object({
  REACT_APP_API_ENDPOINT: z.string().url(),
  // ... more vars to validate
});

envVariables.parse(process.env); // this fails
niksauer commented 1 year ago

Running Storybook 7.0.12 with CRA , using a variable to read an environment value is not working.

const name = "REACT_APP_EXAMPLE";

console.log(process.env.REACT_APP_EXAMPLE);    // ✅ logs real value
console.log(process.env["REACT_APP_EXAMPLE"]); // ✅ logs real value
console.log(process.env[name]);                // ❌ logs undefined
console.log(process.env);                      // ✅ logs non-empty object

Will this ever be worked on? Should be a simple fix to allow dynamic access 😐

niksauer commented 1 year ago

I'm pretty sure it's due to these changes: https://github.com/storybookjs/storybook/pull/17174/files

niksauer commented 1 year ago

Following the above changes, I was able to retain dynamic environment access:

// main.ts
import type { StorybookConfig } from '@storybook/react-webpack5';

const config: StorybookConfig = {
  [...],
  env: (config) => ({
    ...config,
    STORYBOOK_ENVS: JSON.stringify(config),
  }),
};

export default config;
// environment.ts
function getEnv<IsOptional extends boolean = false>(
  name: string,
  isOptional?: IsOptional
): Env<string, IsOptional> {
  let environment: Record<string, unknown>;

  if (process.env.STORYBOOK === 'true') {
    assert(
      process.env.STORYBOOK_ENVS !== undefined,
      `Missing required environment variable: STORYBOOK_ENVS`
    );
    environment = JSON.parse(process.env.STORYBOOK_ENVS);
  } else {
    environment = process.env;
  }

  const processValue = environment[`REACT_APP_${name}`];
  const windowValue = (window as any)[name];
  const value = processValue || windowValue; // prefer local development override

  if (!isOptional) {
    assert(
      value !== undefined && value !== `$${name}`,
      `Missing required environment variable: ${name}`
    );
  }

  return value;
}
cherouvim commented 1 year ago

Confirming issue with Storybook 6.5.16 and 7.4.0.

My temp workaround is to set whatever I need in main.js:

module.exports = {
    // ...
    env: config => ({
        ...config,
        REACT_FOOBAR: "whatever"
    })
};
riceball1 commented 1 month ago

Not sure, if this will help anyone, but instead of using $env/dynamic/public I was able to get things working with import.meta.env