storybookjs / storybook

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

[Bug]: Automigrating SvelteKit projects fails because CJS requires ESM-only modules #21208

Closed JReinhold closed 1 year ago

JReinhold commented 1 year ago

Describe the bug

Recent changes to the automigration script architecture causes automigrations from fresh SvelteKit+Storybook 6.5 projects to fail with the following error:

[Storybook automigrate] ❌ Failed trying to evaluate .storybook/main.cjs with the following error: No "exports" main defined in REPO/node_modules/@sveltejs/adapter-auto/package.json
Please fix the error and try again. 

A fresh Svelte+Storybook 6.5 project initialises with the following main.cjs:

// .storybook/main.cjs

module.exports = {
    stories: [
        "../src/**/*.stories.mdx",
        "../src/**/*.stories.@(js|jsx|ts|tsx|svelte)",
    ],
    addons: [
        "@storybook/addon-links",
        "@storybook/addon-essentials",
        "@storybook/addon-interactions",
    ],
    framework: "@storybook/svelte",
    core: {
        builder: "@storybook/builder-vite",
    },
    svelteOptions: {
        preprocess: require("../svelte.config.js").preprocess,
    },
    features: {
        storyStoreV7: true,
    },
};

The require("../svelte.config.js) call imports the following ESM file (which I believe is valid to do):

// svelte.config.js

import adapter from '@sveltejs/adapter-auto';

/** @type {import('@sveltejs/kit').Config} */
const config = {
    kit: {
        adapter: adapter()
    }
};

export default config;

But @sveltejs/adapter-auto is ESM-only, so it only has the following export map: https://github.com/sveltejs/kit/blob/master/packages/adapter-auto/package.json#L12-L18

"exports": {
  ".": {
    "types": "./index.d.ts",
    "import": "./index.js"
  },
  "./package.json": "./package.json"
}

I believe that causes the error.

Converting main.cjs to ESM makes the error go away (and moves you on the next error described in https://github.com/storybookjs/storybook/issues/21209):

// .storybook/main.js

export default {
    stories: [
        "../src/**/*.stories.mdx",
        "../src/**/*.stories.@(js|jsx|ts|tsx|svelte)",
    ],
    addons: [
        "@storybook/addon-links",
        "@storybook/addon-essentials",
        "@storybook/addon-interactions",
    ],
    framework: "@storybook/svelte",
    core: {
        builder: "@storybook/builder-vite",
    },
    svelteOptions: {
        preprocess: import("../svelte.config.js").preprocess,
    },
    features: {
        storyStoreV7: true,
    },
};

To Reproduce

# create dir
mkdir sb--issue-21208
# init SvelteKit
npm create svelte-with-args@latest -- --directory=. --name=sb-issue-21208 --template=skeleton --types=null --no-prettier --no-eslint --no-playwright --no-vitest
# init Storybook 6.5
npx sb init
# trigger failing automigration
npx sb@next upgrade --prerelease

System

No response

Additional context

No response

JReinhold commented 1 year ago

This might not be an issue after all. After thinking about it and experimenting, it seems like the current CJS+ESM setup in a fresh SvelteKit+SB 6.5 project doesn't even work properly, it fails with similar CJS+ESM issues out of the gate.

And if the setup doesn't even work in a 6.5 project to begin with, I don't think anyone can expect that upgrading or automigrations works either. I think it's good to scope automigrations to actually only work on projects that already work.