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

fix(tools): update migrate-converged-pkg with latest changes so its usable again #20386

Merged
merged 2 commits into from
Oct 27, 2021
Merged

fix(tools): update migrate-converged-pkg with latest changes so its usable again #20386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8902e8a
fix(tools): remove caret when adding babel make styles dependency
Hotell Oct 26, 2021
f6b0622
refactor(tools): remove all related logic to migrating react-examples…
Hotell Oct 27, 2021
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: 0 additions & 4 deletions tools/generators/migrate-converged-pkg/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ Workspace Generator for migrating converged packages to new DX (stage 1)[https:/

## NOTES
Copy link
Member

Choose a reason for hiding this comment

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

Maybe remove this heading too?


- this generator will migrate existing stories for your package from `react-examples` to your migrated package
- migrated stories source will be transformed (replacing absolute imports and adding required default export)
- if your original stories (within react-examples) used non converged packages/tools (like old icons, css/scss for styling), you'll need to manually refactor your stories to remove any non converged dependencies.

## Usage

```sh
Expand Down
190 changes: 11 additions & 179 deletions tools/generators/migrate-converged-pkg/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,6 @@ describe('migrate-converged-pkg generator', () => {
}`,
);
tree = setupDummyPackage(tree, options);
tree = setupDummyPackage(tree, {
name: '@proj/react-examples',
version: '8.0.0',
dependencies: {
[options.name]: '9.0.40-alpha1',
'@proj/old-v8-foo': '8.0.40',
'@proj/old-v8-bar': '8.0.41',
},
});
tree = setupDummyPackage(tree, {
name: '@proj/babel-make-styles',
version: '9.0.0-alpha.0',
Expand Down Expand Up @@ -403,82 +394,47 @@ describe('migrate-converged-pkg generator', () => {
});

describe(`storybook updates`, () => {
function setup(_config?: Partial<{ createDummyStories: boolean }>) {
const defaults = { createDummyStories: true };
const config = { ...defaults, ..._config };

function setup(config: Partial<{ createDummyStories: boolean }> = {}) {
const workspaceConfig = readWorkspaceConfiguration(tree);
const projectConfig = readProjectConfiguration(tree, options.name);
const normalizedProjectName = options.name.replace(`@${workspaceConfig.npmScope}/`, '');
const reactExamplesConfig = readProjectConfiguration(tree, '@proj/react-examples');
const pathToStoriesWithinReactExamples = `${reactExamplesConfig.root}/src/${normalizedProjectName}`;
const projectStorybookConfigPath = `${projectConfig.root}/.storybook`;

const normalizedProjectNameNamesVariants = names(normalizedProjectName);

const paths = {
reactExamples: {
/* eslint-disable @fluentui/max-len */
// options.name==='@proj/react-dummy' -> react-examples/src/react-dummy/ReactDummyOther/ReactDummy.stories.tsx
storyFileOne: `${pathToStoriesWithinReactExamples}/${normalizedProjectNameNamesVariants.className}/${normalizedProjectNameNamesVariants.className}.stories.tsx`,
// if options.name==='@proj/react-dummy' -> react-examples/src/react-dummy/ReactDummyOther/ReactDummyOther.stories.tsx
storyFileTwo: `${pathToStoriesWithinReactExamples}/${normalizedProjectNameNamesVariants.className}Other/${normalizedProjectNameNamesVariants.className}Other.stories.tsx`,
/* eslint-enable @fluentui/max-len */
},
storyOne: `${projectConfig.root}/src/${normalizedProjectNameNamesVariants.className}.stories.tsx`,
Copy link
Member

Choose a reason for hiding this comment

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

I think stories are supposed to go under src/stories now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

q: do we have this documented ? I noticed some packages moved it there but was missing context/reasoning. ty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also in general it is not important where they are - having implementation with test coverage tightly coupled on details is not a good testing pattern.

storyTwo: `${projectConfig.root}/src/${normalizedProjectNameNamesVariants.className}Other.stories.tsx`,
};

if (config.createDummyStories) {
tree.write(
paths.reactExamples.storyFileOne,
paths.storyOne,
stripIndents`
import * as Implementation from '${options.name}';
import * as Implementation from './index';
export const Foo = (props: FooProps) => { return <div>Foo</div>; }
`,
);

tree.write(
paths.reactExamples.storyFileTwo,
paths.storyTwo,
stripIndents`
import * as Implementation from '${options.name}';
import * as Implementation from './index';
export const FooOther = (props: FooPropsOther) => { return <div>FooOther</div>; }
`,
);
}

function getMovedStoriesData() {
const movedStoriesExportNames = {
storyOne: `${normalizedProjectNameNamesVariants.className}`,
storyTwo: `${normalizedProjectNameNamesVariants.className}Other`,
};
const movedStoriesFileNames = {
storyOne: `${movedStoriesExportNames.storyOne}.stories.tsx`,
storyTwo: `${movedStoriesExportNames.storyTwo}.stories.tsx`,
};
const movedStoriesPaths = {
storyOne: `${projectConfig.root}/src/${movedStoriesFileNames.storyOne}`,
storyTwo: `${projectConfig.root}/src/${movedStoriesFileNames.storyTwo}`,
};

const movedStoriesContent = {
storyOne: tree.read(movedStoriesPaths.storyOne)?.toString('utf-8'),
storyTwo: tree.read(movedStoriesPaths.storyTwo)?.toString('utf-8'),
};

return { movedStoriesPaths, movedStoriesExportNames, movedStoriesFileNames, movedStoriesContent };
}

return {
projectConfig,
reactExamplesConfig,
workspaceConfig,
normalizedProjectName,
pathToStoriesWithinReactExamples,
getMovedStoriesData,
projectStorybookConfigPath,
};
}
it(`should setup package storybook when needed`, async () => {
const { projectStorybookConfigPath, projectConfig } = setup();
const { projectStorybookConfigPath, projectConfig } = setup({ createDummyStories: true });

console.error({ projectConfig });

expect(tree.exists(projectStorybookConfigPath)).toBeFalsy();

Expand Down Expand Up @@ -572,128 +528,6 @@ describe('migrate-converged-pkg generator', () => {
'storybook__addons',
);
});

it(`should work if there are no package stories in react-examples`, async () => {
const reactExamplesConfig = readProjectConfiguration(tree, '@proj/react-examples');
const workspaceConfig = readWorkspaceConfiguration(tree);

expect(
tree.exists(`${reactExamplesConfig.root}/src/${options.name.replace(`@${workspaceConfig.npmScope}/`, '')}`),
).toBe(false);

const loggerWarnSpy = jest.spyOn(logger, 'warn');
let sideEffectsCallback: () => void;

try {
sideEffectsCallback = await generator(tree, options);
sideEffectsCallback();
} catch (err) {
expect(err).toEqual(undefined);
}

expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
expect(loggerWarnSpy).toHaveBeenCalledWith(
'No package stories found within react-examples. Skipping storybook stories migration...',
);
});

it(`should move stories from react-examples package to local package within sourceRoot`, async () => {
const { pathToStoriesWithinReactExamples, getMovedStoriesData } = setup();

const loggerWarnSpy = jest.spyOn(logger, 'warn');

expect(tree.exists(pathToStoriesWithinReactExamples)).toBeTruthy();

const sideEffectsCallback = await generator(tree, options);

const { movedStoriesPaths } = getMovedStoriesData();

expect(tree.exists(movedStoriesPaths.storyOne)).toBe(true);
expect(tree.exists(movedStoriesPaths.storyTwo)).toBe(true);

sideEffectsCallback();

expect(loggerWarnSpy).toHaveBeenCalledTimes(2);
expect(loggerWarnSpy.mock.calls[0][0]).toEqual('NOTE: Deleting packages/react-examples/src/react-dummy');
expect(loggerWarnSpy.mock.calls[1][0]).toEqual(
expect.stringContaining('- Please update your moved stories to follow standard storybook format'),
);
});

it(`should delete migrated package folder in react-examples`, async () => {
const { pathToStoriesWithinReactExamples, reactExamplesConfig } = setup();

expect(tree.exists(pathToStoriesWithinReactExamples)).toBeTruthy();

await generator(tree, options);

expect(tree.exists(`${reactExamplesConfig.root}/src/${options.name}`)).toBe(false);
});

it(`should replace absolute import path with relative one from index.ts`, async () => {
const { pathToStoriesWithinReactExamples, getMovedStoriesData } = setup();

expect(tree.exists(pathToStoriesWithinReactExamples)).toBeTruthy();

await generator(tree, options);

const { movedStoriesContent } = getMovedStoriesData();

expect(movedStoriesContent.storyOne).not.toContain(options.name);
expect(movedStoriesContent.storyTwo).not.toContain(options.name);

expect(movedStoriesContent.storyOne).toContain('./index');
expect(movedStoriesContent.storyTwo).toContain('./index');
});

it(`should append storybook CSF default export`, async () => {
const { pathToStoriesWithinReactExamples, getMovedStoriesData } = setup();

expect(tree.exists(pathToStoriesWithinReactExamples)).toBeTruthy();

await generator(tree, options);

const { movedStoriesExportNames, movedStoriesContent } = getMovedStoriesData();

expect(movedStoriesContent.storyOne).toContain(
stripIndents`
export default {
title: 'Components/${movedStoriesExportNames.storyOne}',
component: ${movedStoriesExportNames.storyOne},
}
`,
);
expect(movedStoriesContent.storyTwo).toContain(
stripIndents`
export default {
title: 'Components/${movedStoriesExportNames.storyTwo}',
component: ${movedStoriesExportNames.storyTwo},
}
`,
);
});

it(`should remove package-dependency from react-examples package.json`, async () => {
const { reactExamplesConfig } = setup();

let reactExamplesPkgJson = readJson<PackageJson>(tree, `${reactExamplesConfig.root}/package.json`);

expect(reactExamplesPkgJson.dependencies).toEqual(
expect.objectContaining({
[options.name]: expect.any(String),
}),
);

await generator(tree, options);

reactExamplesPkgJson = readJson<PackageJson>(tree, `${reactExamplesConfig.root}/package.json`);

expect(reactExamplesPkgJson.dependencies).not.toEqual(
expect.objectContaining({
[options.name]: expect.any(String),
}),
);
});
});

describe('package.json updates', () => {
Expand Down Expand Up @@ -890,7 +724,7 @@ describe('migrate-converged-pkg generator', () => {
'@babel/transform-react-pure-annotations',
],
});
expect(devDeps[babelMakeStylesPkg]).toBe('^9.0.0-alpha.0');
expect(devDeps[babelMakeStylesPkg]).toBe('9.0.0-alpha.0');
});

it(`should add @fluentui/babel-make-styles plugin only if needed`, async () => {
Expand Down Expand Up @@ -1042,7 +876,6 @@ describe('migrate-converged-pkg generator', () => {
it(`should run migration on all vNext packages in batch`, async () => {
const projects = [
options.name,
'@proj/react-examples',
'@proj/react-foo',
'@proj/react-bar',
'@proj/react-moo',
Expand All @@ -1062,7 +895,6 @@ describe('migrate-converged-pkg generator', () => {
expect(configs['@proj/react-moo'].sourceRoot).toBeDefined();
expect(configs['@proj/react-dummy'].sourceRoot).toBeDefined();
expect(configs['@proj/react-old'].sourceRoot).not.toBeDefined();
expect(configs['@proj/react-examples'].sourceRoot).not.toBeDefined();
});
});
});
Expand Down
Loading