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

Svelte-vite: remove svelteOptions in automigration #20270

Merged
merged 4 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions code/frameworks/sveltekit/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ npx storybook@next upgrade --prerelease

When running the `upgrade` command above you should get a prompt asking you to migrate to `@storybook/sveltekit`, which should handle everything for you. In some cases it can't migrate for you, eg. if your existing Storybook setup is based on Webpack. In such cases, refer to the manual migration below.

Storybook 7.0 automatically loads your Vite config, and by extension your Svelte config. If you have a `svelteOptions` property in `.storybook/main.cjs` you need to remove that. See [Troubleshooting](#error-about-__esbuild_register_import_meta_url__-when-starting-storybook) below. We're working on doing this automatically soon.
Storybook 7.0 automatically loads your Vite config, and by extension your Svelte config. If you had a `svelteOptions` property in `.storybook/main.cjs` the automigration will have removed it, as it is no longer supported.

#### Manual migration

Expand Down Expand Up @@ -109,7 +109,7 @@ yarn remove @storybook/builder-vite
> ERR! SyntaxError: Identifier '__esbuild_register_import_meta_url__' has already been declared
> ```

You'll get this error when upgrading from 6.5 to 7.0. You need to remove the `svelteOptions` property in `.storybook/main.cjs`, as that is not supported by Storybook 7.0 + SvelteKit. The property is also not necessary anymore because the Vite and Svelte configurations are loaded automatically in Storybook 7.0.
You'll get this error when manually upgrading from 6.5 to 7.0. You need to remove the `svelteOptions` property in `.storybook/main.cjs`, as that is not supported by Storybook 7.0 + SvelteKit. The property is also not necessary anymore because the Vite and Svelte configurations are loaded automatically in Storybook 7.0.

## Acknowledgements

Expand Down
9 changes: 8 additions & 1 deletion code/lib/cli/src/automigrate/fixes/new-frameworks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ export const newFrameworks: Fix<NewFrameworkRunOptions> = {
return null;
}

const frameworkOptions = getFrameworkOptions(framework, main);
const frameworkOptions =
// svelte-vite doesn't support svelteOptions so there's no need to move them
newFrameworkPackage === '@storybook/svelte-vite' ? {} : getFrameworkOptions(framework, main);

const dependenciesToRemove = [
'@storybook/builder-webpack5',
Expand Down Expand Up @@ -256,6 +258,11 @@ export const newFrameworks: Fix<NewFrameworkRunOptions> = {
delete currentCore.builder;
}

if (frameworkPackage === '@storybook/svelte-vite' && main.getFieldNode(['svelteOptions'])) {
logger.info(`✅ Removing svelteOptions field in main.js`);
main.removeField(['svelteOptions']);
}

if (Object.keys(builderInfo.options).length > 0) {
main.setFieldValue(['framework', 'options', 'builder'], builderInfo.options);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ describe('SvelteKit framework fix', () => {
await expect(checkSvelteKitFramework({ packageJson, main })).resolves.toMatchObject({
packageJson,
main: expect.objectContaining({}),
frameworkOptions: undefined,
dependenciesToRemove: ['@storybook/svelte-vite'],
});
});
Expand All @@ -146,7 +145,6 @@ describe('SvelteKit framework fix', () => {
await expect(checkSvelteKitFramework({ packageJson, main })).resolves.toMatchObject({
packageJson,
main: expect.objectContaining({}),
frameworkOptions: undefined,
dependenciesToRemove: ['@storybook/builder-vite'],
});
});
Expand All @@ -166,7 +164,6 @@ describe('SvelteKit framework fix', () => {
await expect(checkSvelteKitFramework({ packageJson, main })).resolves.toMatchObject({
packageJson,
main: expect.objectContaining({}),
frameworkOptions: undefined,
dependenciesToRemove: ['storybook-builder-vite'],
});
});
Expand Down
22 changes: 7 additions & 15 deletions code/lib/cli/src/automigrate/fixes/sveltekit-framework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const logger = console;
interface SvelteKitFrameworkRunOptions {
main: ConfigFile;
packageJson: PackageJsonWithDepsAndDevDeps;
frameworkOptions: Record<string, any> | undefined;
dependenciesToRemove: string[];
}

Expand Down Expand Up @@ -70,7 +69,6 @@ export const sveltekitFramework: Fix<SvelteKitFrameworkRunOptions> = {
}

const framework = typeof frameworkConfig === 'string' ? frameworkConfig : frameworkConfig.name;
const frameworkOptions = main.getFieldValue(['framework', 'options']);

if (framework === '@storybook/sveltekit') {
// already using the new framework
Expand All @@ -81,7 +79,6 @@ export const sveltekitFramework: Fix<SvelteKitFrameworkRunOptions> = {
// direct migration from svelte-vite projects
return {
main,
frameworkOptions,
packageJson,
dependenciesToRemove: ['@storybook/svelte-vite'],
};
Expand Down Expand Up @@ -125,7 +122,6 @@ export const sveltekitFramework: Fix<SvelteKitFrameworkRunOptions> = {
// migration from 6.x projects using Svelte with the Vite builder
return {
main,
frameworkOptions,
packageJson,
dependenciesToRemove: [builder],
};
Expand All @@ -146,11 +142,7 @@ export const sveltekitFramework: Fix<SvelteKitFrameworkRunOptions> = {
`;
},

async run({
result: { main, frameworkOptions, packageJson, dependenciesToRemove },
packageManager,
dryRun,
}) {
async run({ result: { main, packageJson, dependenciesToRemove }, packageManager, dryRun }) {
logger.info(`✅ Removing redundant packages: ${dependenciesToRemove.join(', ')}`);
if (!dryRun) {
packageManager.removeDependencies({ skipInstall: true, packageJson }, dependenciesToRemove);
Expand All @@ -165,12 +157,7 @@ export const sveltekitFramework: Fix<SvelteKitFrameworkRunOptions> = {
}

logger.info(`✅ Updating framework field in main.js`);
if (frameworkOptions) {
main.setFieldValue(['framework', 'options'], frameworkOptions);
main.setFieldValue(['framework', 'name'], '@storybook/sveltekit');
} else {
main.setFieldValue(['framework'], '@storybook/sveltekit');
}
main.setFieldValue(['framework'], '@storybook/sveltekit');

const currentCore = main.getFieldValue(['core']);
if (currentCore.builder) {
Expand All @@ -184,6 +171,11 @@ export const sveltekitFramework: Fix<SvelteKitFrameworkRunOptions> = {
}
}

if (main.getFieldNode(['svelteOptions'])) {
logger.info(`✅ Removing svelteOptions field in main.js`);
main.removeField(['svelteOptions']);
}

if (!dryRun) {
await writeConfig(main);
}
Expand Down