Core: Add deprecation notice for Vite + CommonJS

[JReinhold]

What I did

Vite v5 will remove support for CommonJS, so we plan to remove support for main.cjs and any CommonJS syntax within (regardless of file extension) in Storybook 8 with Vite. This won't guarantee that Vite 5 works, but it's at least a step in the right direction.

Technically this doesn't make much of a difference because main.js isn't loaded by Vite, but rather by esbuild-register because it's server-only, but I think it's a good upgrade for users.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

• stories
• unit tests
• integration tests
• end-to-end tests

Manual testing

I manually tested this in a React-Vite sandbox by renaming the main.js file to main.cjs and main.cts, which caused the deprecation warning to appear. It also correctly appears if either module.exports = config or exports.default = config is used. It correctly does not appear in a plain main.js file with export default config;.

Documentation

• Add or update documentation reflecting your changes
• If you are deprecating/removing a feature, make sure to update
  MIGRATION.MD

Checklist for Maintainers

• When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

• Make sure this PR contains one of the labels below:

  Available labels

  • bug: Internal changes that fixes incorrect behavior.
  • maintenance: User-facing maintenance tasks.
  • dependencies: Upgrading (sometimes downgrading) dependencies.
  • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
  • cleanup: Minor cleanup style change. Will not show up in release changelog.
  • documentation: Documentation only changes. Will not show up in release changelog.
  • feature request: Introducing a new feature.
  • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
  • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

[JReinhold]

@benmccann @IanVS, @ndelangen reminded me that main.js isn't even loaded by Vite because it's only run on the server, so it's loaded by esbuild-register, so I wouldn't expect this to break even if Vite stopped supporting CJS.

I do think we should deprecate this anyway, as it's a low cost migration for the user.

[shilman]

I'm not opposed to these changes, but why do we need to do it at all? Vite is not processing main.js at all -- that's Storybook's configuration file.

Sorry, I saw you answered my question above. Perhaps this is a good chance to upgrade users to the new configuration format with the default export?

[benmccann]

main.js isn't even loaded by Vite because it's only run on the server, so it's loaded by esbuild-register, so I wouldn't expect this to break even if Vite stopped supporting CJS.

It doesn't really matter what loads the main.js, but actually taking a second look at this I think it might not be an issue for a different reason, which is that the main.js doesn't actually include any code itself, but only uses string references such as framework: "@storybook/svelte-vite". I'm afraid I'd forgotten that as I'm used to config files in a lot of projects directly importing the packages.

There is still potentially an issue here regardless of what handles main.js. This issue isn't about Vite processing anything, but rather it's about Vite itself being distributed as ESM-only. If anything tries to require Vite then it will fail. It will need to be included via an import. So I guess we need to figure out what loads the framework specified in the config file (rather than changing the config file itself as we're doing here) and ensure it's using import rather than require.

[benmccann]

I don't know if we want to continue with this PR or not, but if we do then it will need a rebase.

I do think we should deprecate this anyway, as it's a low cost migration for the user.

If we do want to make this change as simply a good practice, should we do it for all users rather than just Vite users then since we've realized this isn't actually required by Vite?

[ndelangen]

main.js isn't even loaded by Vite because it's only run on the server, so it's loaded by esbuild-register, so I wouldn't expect this to break even if Vite stopped supporting CJS.

It doesn't really matter what loads the main.js, but actually taking a second look at this I think it might not be an issue for a different reason, which is that the main.js doesn't actually include any code itself, but only uses string references such as framework: "@storybook/svelte-vite". I'm afraid I'd forgotten that as I'm used to config files in a lot of projects directly importing the packages.

There is still potentially an issue here regardless of what handles main.js. This issue isn't about Vite processing anything, but rather it's about Vite itself being distributed as ESM-only. If anything tries to require Vite then it will fail. It will need to be included via an import. So I guess we need to figure out what loads the framework specified in the config file (rather than changing the config file itself as we're doing here) and ensure it's using import rather than require.

Some day Storybook will also follow that pattern, where we expect the user to import addons rather than reference them by string, FYI.

Possible in 9.0