Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Vite CJS warning still present in v8 RC #26291

Open
dan-mba opened this issue Mar 3, 2024 · 19 comments
Open

[Bug]: Vite CJS warning still present in v8 RC #26291

dan-mba opened this issue Mar 3, 2024 · 19 comments

Comments

@dan-mba
Copy link

dan-mba commented Mar 3, 2024

Describe the bug

Using Vite 5 with Storybook 8 RC produces the following warning:

The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details

According to #24333 this was supposed to be fixed with v8.
As a RC has been released, I would assume this should have been fixed by now.

To Reproduce

https://github.com/dan-mba/react-svg-components/tree/sb-8

System

Storybook Environment Info:

  System:
    OS: Linux 5.15 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish)
    CPU: (16) x64 AMD Ryzen 7 5700G with Radeon Graphics
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 20.11.1 - ~/.nvm/versions/node/v20.11.1/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v20.11.1/bin/yarn
    npm: 10.4.0 - ~/.nvm/versions/node/v20.11.1/bin/npm <----- active
    pnpm: 8.7.0 - ~/.nvm/versions/node/v20.11.1/bin/pnpm
  Browsers:
    Chrome: 122.0.6261.94
  npmPackages:
    @storybook/addon-essentials: ^8.0.0-rc.1 => 8.0.0-rc.1
    @storybook/react: ^8.0.0-rc.1 => 8.0.0-rc.1
    @storybook/react-vite: ^8.0.0-rc.1 => 8.0.0-rc.1
    eslint-plugin-storybook: ^0.8.0 => 0.8.0
    storybook: ^8.0.0-rc.1 => 8.0.0-rc.1

Additional context

No response

@codeart1st
Copy link

I also tried 8.0.0-rc.1 for our own setup. Can't confirm this, but have you tried adding this "type": "module" to your package.json?

@Staremang
Copy link

I also tried 8.0.0-rc.1 for our own setup. Can't confirm this, but have you tried adding this "type": "module" to your package.json?

Confirmed. I have the same error

image

@vanessayuenn
Copy link
Contributor

@JReinhold @IanVS any idea why #24395 didn't fully solve this?

@IanVS
Copy link
Member

IanVS commented Mar 4, 2024

Yes, it's because of the way we load the user's config file as CJS (using https://github.com/egoist/esbuild-register), which causes Vite to still show the warning.

@JReinhold
Copy link
Contributor

If users are importing from Vite in their main config like @dan-mba is in their reproduction, they need to use a dynamic import instead of a top-level import to silence this warning, like this:

import type { StorybookConfig } from "@storybook/react-vite";
- import { mergeConfig } from 'vite';

const config: StorybookConfig = {
  stories: ["../src/**/*.stories.@(js|jsx|mjs|ts|tsx)"],
  addons: [ "@storybook/addon-essentials" ],
  framework: {
    name: "@storybook/react-vite",
    options: {},
  },
  docs: {
    autodocs: false,
  },
  async viteFinal(config, {
    configType
  }) {
+    const { mergeConfig } = await import('vite');
    // return the customized config
    return mergeConfig(config, {
      // customize the Vite config here
      base: './'
    });
  },
};
export default config;

This is not trivial to fix for us internally as it requires that we make changes to our preset loading, something @ndelangen is hesitant about and definitely won't happen for 8.0 GA. I think @ndelangen's hope is to do this as part of the ESM transition?

We could perhaps document this dynamic import requirement though? @kylegach

@JReinhold JReinhold removed this from the 8.0-Stable milestone Mar 5, 2024
@dan-mba
Copy link
Author

dan-mba commented Mar 5, 2024

If users are importing from Vite in their main config like @dan-mba is in their reproduction, they need to use a dynamic import instead of a top-level import to silence this warning, like this:

import type { StorybookConfig } from "@storybook/react-vite";
- import { mergeConfig } from 'vite';

const config: StorybookConfig = {
  stories: ["../src/**/*.stories.@(js|jsx|mjs|ts|tsx)"],
  addons: [ "@storybook/addon-essentials" ],
  framework: {
    name: "@storybook/react-vite",
    options: {},
  },
  docs: {
    autodocs: false,
  },
  async viteFinal(config, {
    configType
  }) {
+    const { mergeConfig } = await import('vite');
    // return the customized config
    return mergeConfig(config, {
      // customize the Vite config here
      base: './'
    });
  },
};
export default config;

This is not trivial to fix for us internally as it requires that we make changes to our preset loading, something @ndelangen is hesitant about and definitely won't happen for 8.0 GA. I think @ndelangen's hope is to do this as part of the ESM transition?

We could perhaps document this dynamic import requirement though? @kylegach

@JReinhold The dynamic import got rid of the warning for me.

The sample main.js|ts in the Vite Configuration Docs will cause the same warning.
Adding a mention about dynamic import here would be useful.

It might also be nice to have a warning when using the update migration cli.

@JReinhold
Copy link
Contributor

The sample main.js|ts in the Vite Configuration Docs will cause the same warning. Adding a mention about dynamic import here would be useful.

Good catch, I've updated the snippets to dynamically import in #26330

It might also be nice to have a warning when using the update migration cli.

This is actually unrelated to upgrading to any Storybook version, it's about users upgrading to Vite 5. So I don't think it's appropriate to show in the upgrade CLI flow.

@wuifdesign
Copy link

i still get the warning when using:

typescript: {
    reactDocgen: "react-docgen-typescript",
},

@aimad-majdou
Copy link

aimad-majdou commented Apr 19, 2024

If users are importing from Vite in their main config like @dan-mba is in their reproduction, they need to use a dynamic import instead of a top-level import to silence this warning, like this:

import type { StorybookConfig } from "@storybook/react-vite";
- import { mergeConfig } from 'vite';

const config: StorybookConfig = {
  stories: ["../src/**/*.stories.@(js|jsx|mjs|ts|tsx)"],
  addons: [ "@storybook/addon-essentials" ],
  framework: {
    name: "@storybook/react-vite",
    options: {},
  },
  docs: {
    autodocs: false,
  },
  async viteFinal(config, {
    configType
  }) {
+    const { mergeConfig } = await import('vite');
    // return the customized config
    return mergeConfig(config, {
      // customize the Vite config here
      base: './'
    });
  },
};
export default config;

This is not trivial to fix for us internally as it requires that we make changes to our preset loading, something @ndelangen is hesitant about and definitely won't happen for 8.0 GA. I think @ndelangen's hope is to do this as part of the ESM transition?

We could perhaps document this dynamic import requirement though? @kylegach

We have changed our main.ts accordingly, but we still get the same error, here is how the file looks:

import type { StorybookConfig } from '@storybook/react-vite';

const config: StorybookConfig = {
  stories: ['../src/**/*.mdx', '../src/**/*.stories.@(js|jsx|ts|tsx)'],
  addons: [
    '@storybook/addon-links',
    '@storybook/addon-essentials',
    '@storybook/addon-interactions',
    '@storybook/addon-coverage',
    '@storybook/addon-themes',
    '@storybook/addon-a11y',
  ],
  framework: {
    name: '@storybook/react-vite',
    options: {},
  },
  docs: {
    autodocs: true,
  },
  async viteFinal(config) {
    const { mergeConfig } = await import('vite');
    const viteTsconfig = await import('vite-tsconfig-paths');
    const tsconfigPaths = viteTsconfig.default;
    return mergeConfig(config, {
      plugins: [tsconfigPaths()], // resolve imports using TypeScript's path mapping
    });
  },
  typescript: {
    check: false,
  },
  core: {
    disableTelemetry: true, // Disables telemetry
  },
};
export default config;

@aimad-majdou
Copy link

aimad-majdou commented Apr 19, 2024

Upon tracing the origin of the CJS warning, I identified its source in the file: node_modules/vite-plugin-istanbul/dist/index.cjs:

image

It's worth noting that upon the comment of @wuifdesign, I also removed reactDocgen: "react-docgen-typescript", as it was another trigger for the warning message. As evidenced in this screenshot, the warning comes from node_modules\@joshwooding\vite-plugin-react-docgen-typescript\dist\index.cjs:3:

image

The warning ceased to appear after the removal of both @storybook/addon-coverage and react-docgen-typescript.

Both files share the same import statement: const vite = require('vite');

This import seems to be the culprit behind the warning. Based on my understanding from https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated, it appears that vite should be imported dynamically.


Update:

Upon examination of the vite-plugin-react-docgen-typescript, it appears that the issue has been addressed in version 0.3.1, as evidenced by the changes documented here.

In our project, we resolved this by explicitly setting the version in the package.json file:

"overrides": {
    "@joshwooding/vite-plugin-react-docgen-typescript": "0.3.1"
  }

However, it's worth noting that the issue persists in the vite-plugin-istanbul project. They continue to utilize the normal import for vite, as illustrated here.

I have opened a ticket for that: iFaxity/vite-plugin-istanbul#209

@vanessayuenn
Copy link
Contributor

Looks like this issue has been resolved upstream, so closing this now. Let me know if this isn't the case!

@zuk-michal
Copy link

@vanessayuenn It's not solved yet for this case:

typescript: {
    reactDocgen: "react-docgen-typescript",
},

Copy link
Contributor

greptile-apps bot commented Jul 20, 2024

** Disclaimer** This information might be inaccurate, due to it being generated automatically
To address the Vite CJS warning in Storybook 8 RC, update the Vite configuration to use the ESM build of Vite's Node API. Modify the vite.config.ts file in the /code/builders/builder-vite/src directory as follows:

import { defineConfig } from 'vite';

export default defineConfig({
  // existing configuration
  optimizeDeps: {
    esbuildOptions: {
      target: 'esnext',
      format: 'esm',
    },
  },
});

Ensure that the vite dependency in package.json is updated to a version that supports ESM:

"vite": "^5.0.0"

References

/.github/DISCUSSION_TEMPLATE/help.yml
/code/builders/builder-vite/README.md
/code/lib/cli/src/automigrate/fixes/addon-postcss.ts
/code/lib/cli/src/autoblock/block-storystorev6.ts
/code/lib/cli/src/automigrate/fixes/cra5.ts
/code/lib/cli/src/automigrate/fixes/vue3.ts
/code/lib/cli/src/automigrate/fixes/prompt-remove-react.ts
/code/core/src/server-errors.ts
/.github/comments/invalid-link.md
/code/lib/cli/src/automigrate/fixes/angular-builders.ts
/code/frameworks/svelte-vite/README.md
/code/frameworks/angular/src/builders/build-storybook/schema.json
/docs/versions/next.json
/docs/get-started/frameworks/react-vite.mdx
/code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.ts
/code/builders/builder-vite
/code/builders/builder-vite/package.json
/code/lib/cli/src/automigrate/fixes/builder-vite.test.ts
/code/lib/cli/src/automigrate/fixes/builder-vite.ts
/docs/get-started/frameworks/svelte-vite.mdx
/code/lib/cli/src/automigrate/fixes/webpack5-compiler-setup.ts
/code/lib/cli/src/automigrate/fixes/storyshots-migration.ts
/code/lib/csf-plugin
/code/deprecated/components/package.json
/code/renderers/svelte/package.json

About Greptile

This response provides a starting point for your research, not a precise solution.

Help us improve! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

Ask Greptile · Edit Issue Bot Settings

@kaiyoma
Copy link

kaiyoma commented Jul 20, 2024

I am also seeing this warning, even though my entire repo is ESM (and has been so for over a year). Unlike @zuk-michal I'm disabling doc generation, but changing that setting doesn't seem to matter (I still see the warnings). Our repo is way too large (and proprietary) to provide as a repro, but I'm happy to post config files or logs.

@kaiyoma
Copy link

kaiyoma commented Jul 24, 2024

Will anyone be looking into this soon? To be clear, it's not the RC that has this issue; even the latest version of Storybook (8.2.5) exhibits this. And the issue exists even if doc generation is turned off.

I'm asking because this warning interferes with my monorepo build. Even though the Storybook build works correctly, the presence of the warning causes the monorepo build to not be successful:

==[ SUCCESS WITH WARNINGS: 1 operation ]=======================================

--[ WARNING: storybook-app ]-----------------------[ 4 minutes 14.3 seconds ]--

The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.


Operations succeeded with warnings.

@JReinhold
Copy link
Contributor

@kaiyoma did you try the workaround described in #26291 (comment) ?

If that doesn't work, it would be great to get a minimal reproduction. We are aware of the issue with react-docgen-typescript, but if you're not using that then it would be helpful to understand what is triggering it for you.

@kaiyoma
Copy link

kaiyoma commented Jul 24, 2024

@kaiyoma did you try the workaround described in #26291 (comment) ?

@JReinhold I tried it, but this doesn't help either. Same error. Here's the trace, if this is helpful:

$ VITE_CJS_TRACE=true ds

> [email protected] dev
> cross-env VITE_GEIGER_ENV=storybook storybook dev -c storybook-config --ci --port 9001

storybook v8.2.5

Trace: The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.
    at warnCjsUsage (C:\Users\kgetz\Work\event-viewer\common\temp\node_modules\.pnpm\[email protected]_@[email protected][email protected][email protected]\node_modules\vite\index.cjs:33:3)
    at Object.<anonymous> (C:\Users\kgetz\Work\event-viewer\common\temp\node_modules\.pnpm\[email protected]_@[email protected][email protected][email protected]\node_modules\vite\index.cjs:3:1)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
    at Object.newLoader (C:\Users\kgetz\Work\event-viewer\common\temp\node_modules\.pnpm\[email protected][email protected]\node_modules\esbuild-register\dist\node.js:2262:9)
    at extensions..js (C:\Users\kgetz\Work\event-viewer\common\temp\node_modules\.pnpm\[email protected][email protected]\node_modules\esbuild-register\dist\node.js:4838:24)
    at Module.load (node:internal/modules/cjs/loader:1207:32)
    at Module._load (node:internal/modules/cjs/loader:1023:12)
    at Module.require (node:internal/modules/cjs/loader:1235:19)
    at require (node:internal/modules/helpers:176:18)
info => Starting manager..
info => Starting preview..

I actually just realized that Vite offers an escape hatch for suppressing the warning (VITE_CJS_IGNORE_WARNING=true) so that will unblock me for now. It sounds like this issue will eventually get fixed in Storybook, based on the comments above. Just as long as it gets fixed before Vite removes the CJS API entirely. 😄

@gangsthub
Copy link

gangsthub commented Sep 27, 2024

In my case (Storybook 8.3.3), @storybook/addon-coverage was the reason for this warning. Removing the addon suppresses it.

I tried a similar solution as @aimad-majdou's with [email protected], which is the version they switch to ESM with:

// ❌ not working
{
  "resolutions": {
    "vite-plugin-istanbul": "6.0.1"
  },
}

But this makes the preview (dev environment) fail:

=> Failed to build the preview
Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in ./node_modules/vite-plugin-istanbul/package.json
    at exportsNotFound (node:internal/modules/esm/resolve:304:10)
    at packageExportsResolve (node:internal/modules/esm/resolve:594:13)
    at resolveExports (node:internal/modules/cjs/loader:634:36)
    at Module._findPath (node:internal/modules/cjs/loader:724:31)
    at Module._resolveFilename (node:internal/modules/cjs/loader:1211:27)
    at Module._resolveFilename (./node_modules/@storybook/core/node_modules/esbuild-register/dist/node.js:4799:36)
    at Module._load (node:internal/modules/cjs/loader:1051:27)
    at Module.require (node:internal/modules/cjs/loader:1311:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.viteFinal (./node_modules/@storybook/addon-coverage/dist/cjs/preset.js:19:20)

The main issue is that @storybook/addon-coverage (current version: 1.0.4) uses an ancient version of vite-plugin-istanbul, and needs to be updated (the latest version is at 6.0.2).

> yarn why vite-plugin-istanbul
<
└─ @storybook/addon-coverage@npm:1.0.4
   └─ vite-plugin-istanbul@npm:3.0.4 (via npm:^3.0.1)

I created the issue in @storybook/addon-coverage's repo: storybookjs/addon-coverage#49

@Laruxo
Copy link

Laruxo commented Nov 28, 2024

In my case, this is caused by node_modules/storybook/bin/index.cjs. I see this warning if I import anything from vite inside .storybook/main.ts. Could we add ESM binary to storybook CLI package? Or maybe change the binary to ESM?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests