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.

Sign up for GitHub

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

Automigration: Improve setup file transformation and version range handling for a11y migration #30060

Merged
merged 1 commit into from
Dec 15, 2024
Merged

Automigration: Improve setup file transformation and version range handling for a11y migration #30060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 31 additions & 2 deletions code/lib/cli-storybook/src/automigrate/fixes/addon-a11y-addon-test.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ describe('addonA11yAddonTest', () => {
`;
vi.mocked(readFileSync).mockReturnValue(source);

const transformedCode = transformSetupFile(setupFile);
const s = readFileSync(setupFile, 'utf8');
const transformedCode = transformSetupFile(s);
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
expect(transformedCode).toMatchInlineSnapshot(`
"import * as a11yAddonAnnotations from "@storybook/addon-a11y/preview";
import { beforeAll } from 'vitest';
Expand Down Expand Up @@ -169,7 +170,35 @@ describe('addonA11yAddonTest', () => {
`;
vi.mocked(readFileSync).mockReturnValue(source);

const transformedCode = transformSetupFile(setupFile);
const s = readFileSync(setupFile, 'utf8');
const transformedCode = transformSetupFile(s);
expect(transformedCode).toMatchInlineSnapshot(`
"import * as a11yAddonAnnotations from "@storybook/addon-a11y/preview";
import { beforeAll } from 'vitest';
import { setProjectAnnotations } from 'storybook';
import * as projectAnnotations from './preview';

const project = setProjectAnnotations([a11yAddonAnnotations, projectAnnotations]);

beforeAll(project.beforeAll);"
`);
});

it('should transform setup file correctly - project annotation is not an array', () => {
const setupFile = '/path/to/vitest.setup.ts';
const source = dedent`
import { beforeAll } from 'vitest';
import { setProjectAnnotations } from 'storybook';
import * as projectAnnotations from './preview';

const project = setProjectAnnotations(projectAnnotations);

beforeAll(project.beforeAll);
`;
vi.mocked(readFileSync).mockReturnValue(source);

const s = readFileSync(setupFile, 'utf8');
const transformedCode = transformSetupFile(s);
expect(transformedCode).toMatchInlineSnapshot(`
"import * as a11yAddonAnnotations from "@storybook/addon-a11y/preview";
import { beforeAll } from 'vitest';
Expand Down
23 changes: 15 additions & 8 deletions code/lib/cli-storybook/src/automigrate/fixes/addon-a11y-addon-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ interface AddonA11yAddonTestOptions {
*/
export const addonA11yAddonTest: Fix<AddonA11yAddonTestOptions> = {
id: 'addonA11yAddonTest',
// TODO: Change to the correct version after testing
versionRange: ['<8.5.0', '*'],
versionRange: ['<8.5.0', '>=8.5.0'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: version range ['<8.5.0', '>=8.5.0'] covers all versions and is equivalent to '*'. Consider if this is intentional or if there should be a minimum version requirement.


promptType(result) {
if (result.setupFile === null) {
Expand Down Expand Up @@ -53,7 +52,11 @@ export const addonA11yAddonTest: Fix<AddonA11yAddonTestOptions> = {

try {
if (vitestSetupFile) {
const transformedSetupCode = transformSetupFile(vitestSetupFile);
const source = readFileSync(vitestSetupFile, 'utf8');
if (source.includes('@storybook/addon-a11y')) {
return null;
}
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
const transformedSetupCode = transformSetupFile(source);
return {
setupFile: vitestSetupFile,
transformedSetupCode,
Expand Down Expand Up @@ -124,8 +127,7 @@ export const addonA11yAddonTest: Fix<AddonA11yAddonTestOptions> = {
},
};

export function transformSetupFile(setupFile: string) {
const source = readFileSync(setupFile, 'utf8');
export function transformSetupFile(source: string) {
const j = jscodeshift.withParser('ts');

const root = j(source);
Expand All @@ -148,9 +150,14 @@ export function transformSetupFile(setupFile: string) {
throw new Error('Could not find setProjectAnnotations call in vitest.setup file');
}

// Add a11yAddonAnnotations to the annotations array
setProjectAnnotationsCall.find(j.ArrayExpression).forEach((p) => {
p.value.elements.unshift(j.identifier('a11yAddonAnnotations'));
// Add a11yAddonAnnotations to the annotations array or create a new array if argument is a string
setProjectAnnotationsCall.forEach((p) => {
if (p.value.arguments.length === 1 && p.value.arguments[0].type === 'ArrayExpression') {
p.value.arguments[0].elements.unshift(j.identifier('a11yAddonAnnotations'));
} else if (p.value.arguments.length === 1 && p.value.arguments[0].type === 'Identifier') {
const arg = p.value.arguments[0];
p.value.arguments[0] = j.arrayExpression([j.identifier('a11yAddonAnnotations'), arg]);
}
});
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved

// Add the import declaration at the top
Expand Down
Loading