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

CLI: Add prompt-only automigrate asking for react-removal #25215

Merged
merged 7 commits into from
Dec 18, 2023
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
158 changes: 158 additions & 0 deletions code/lib/cli/src/automigrate/fixes/prompt-remove-react.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
import type { StorybookConfig } from '@storybook/types';
import { glob } from 'glob';
import { removeReactDependency } from './prompt-remove-react';
import type { JsPackageManager } from '../../js-package-manager';

const check = async ({
packageManagerContent,
main: mainConfig,
storybookVersion = '8.0.0',
}: {
packageManagerContent: Pick<
Partial<Awaited<ReturnType<JsPackageManager['retrievePackageJson']>>>,
'dependencies' | 'devDependencies' | 'peerDependencies'
>;
main: Partial<StorybookConfig> & Record<string, unknown>;
storybookVersion?: string;
}) => {
const packageManager = {
retrievePackageJson: async () => packageManagerContent,
} as JsPackageManager;

return removeReactDependency.check({
packageManager,
configDir: '',
mainConfig: mainConfig as any,
storybookVersion,
});
};

jest.mock('glob', () => ({ glob: jest.fn(() => []) }));

describe('early exits', () => {
test('cancel if storybookVersion < 8', async () => {
await expect(
check({
packageManagerContent: {
dependencies: { react: '16.0.0' },
},
main: {
stories: [],
framework: '@storybook/vue-vite',
},
storybookVersion: '7.0.0',
})
).resolves.toBeFalsy();
});

test('cancel if no react deps', async () => {
await expect(
check({
packageManagerContent: {},
main: {
stories: [],
framework: '@storybook/vue-vite',
},
})
).resolves.toBeFalsy();
});

test('cancel if react renderer', async () => {
await expect(
check({
packageManagerContent: {
dependencies: { react: '16.0.0' },
},
main: {
stories: [],
framework: '@storybook/react-vite',
},
})
).resolves.toBeFalsy();

await expect(
check({
packageManagerContent: {
dependencies: { react: '16.0.0' },
},
main: {
stories: [],
framework: '@storybook/nextjs',
},
})
).resolves.toBeFalsy();

await expect(
check({
packageManagerContent: {
dependencies: { react: '16.0.0' },
},
main: {
stories: [],
framework: { name: '@storybook/react-webpack5' },
},
})
).resolves.toBeFalsy();
});
});

describe('prompts', () => {
test('simple', async () => {
await expect(
check({
packageManagerContent: {
dependencies: { react: '16.0.0' },
},
main: {
stories: ['*.stories.ts'],
addons: [],
framework: '@storybook/vue-vite',
},
})
).resolves.toEqual(true);
});
test('detects addon docs', async () => {
await expect(
check({
packageManagerContent: {
dependencies: { react: '16.0.0' },
},
main: {
stories: ['*.stories.ts'],
addons: ['@storybook/addon-docs'],
framework: '@storybook/vue-vite',
},
})
).resolves.toEqual(true);
});
test('detects addon essentials', async () => {
await expect(
check({
packageManagerContent: {
dependencies: { react: '16.0.0' },
},
main: {
stories: ['*.stories.ts'],
addons: ['@storybook/addon-docs', '@storybook/addon-essentials'],
framework: '@storybook/vue-vite',
},
})
).resolves.toEqual(true);
});
test('detects MDX usage', async () => {
// @ts-expect-error (jest mocked)
glob.mockImplementationOnce(() => ['*.stories.mdx']);
await expect(
check({
packageManagerContent: {
dependencies: { react: '16.0.0' },
},
main: {
stories: ['*.stories.ts'],
addons: ['@storybook/addon-docs', '@storybook/addon-essentials'],
framework: '@storybook/vue-vite',
},
})
).resolves.toEqual(true);
});
});
46 changes: 46 additions & 0 deletions code/lib/cli/src/automigrate/fixes/prompt-remove-react.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import dedent from 'ts-dedent';
import semver from 'semver';
import { getFrameworkPackageName } from '../helpers/mainConfigFile';
import type { Fix } from '../types';

export const removeReactDependency: Fix<{}> = {
id: 'remove-react-dependency',

async check({ packageManager, mainConfig, storybookVersion }) {
// when the user is using the react renderer, we should not prompt them to remove react
const frameworkPackageName = getFrameworkPackageName(mainConfig);
if (frameworkPackageName?.includes('react') || frameworkPackageName?.includes('nextjs')) {
return null;
}

// if the user has no dependency on react, we can skip this fix
const packageJson = await packageManager.retrievePackageJson();
if (
!packageJson?.dependencies?.['react'] &&
!packageJson?.peerDependencies?.['react'] &&
!packageJson?.devDependencies?.['react']
) {
return null;
}

// do not prompt to remove react for older versions of storybook
if (!semver.gte(storybookVersion, '8.0.0')) {
return null;
}

return true;
},
prompt() {
return dedent`
We detected that your project has a dependency for "react" that it might not need.
Nothing breaks by having it, you can safely ignore this message, if you wish.
ndelangen marked this conversation as resolved.
Show resolved Hide resolved

Storybook asked you to add "react" as a direct dependency in the past.
However, since version 8.0, Storybook no longer requires you to provide "react" as a dependency.
Some community addons might still wrongfully list "react" and "react-dom" as required peer dependencies, but since Storybook 7.6 it should not be needed in the majority of cases.

If you know you are not using React outside of Storybook, it should be safe to remove the "react" and "react-dom" dependencies from your project's package.json.
Storybook cannot do this automatically as removing it might break your project, so it should be done manually with care.
`;
},
};