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

feat(tools): add --stats flag and update nx workspace config #18483

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
72 changes: 59 additions & 13 deletions tools/generators/migrate-converged-pkg/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,24 @@ import { PackageJson, TsConfig } from '../../types';
import generator from './index';
import { MigrateConvergedPkgGeneratorSchema } from './schema';

interface AssertedSchema extends MigrateConvergedPkgGeneratorSchema {
name: string;
}

describe('migrate-converged-pkg generator', () => {
// eslint-disable-next-line @typescript-eslint/no-empty-function
const noop = () => {};
const originalConsoleLog = {
log: console.log,
warn: console.warn,
};
console.log = noop;
console.warn = noop;

let tree: Tree;
const options: MigrateConvergedPkgGeneratorSchema = { name: '@proj/react-dummy' };
const options = { name: '@proj/react-dummy' };

beforeEach(() => {
jest.restoreAllMocks();

jest.spyOn(console, 'log').mockImplementation(noop);
jest.spyOn(console, 'info').mockImplementation(noop);
jest.spyOn(console, 'warn').mockImplementation(noop);

tree = createTreeWithEmptyWorkspace();
tree.write(
'jest.config.js',
Expand All @@ -52,11 +54,6 @@ describe('migrate-converged-pkg generator', () => {
});
});

afterAll(() => {
console.log = originalConsoleLog.log;
console.warn = originalConsoleLog.warn;
});

describe('general', () => {
it(`should throw error if name is empty`, async () => {
await expect(generator(tree, { name: '' })).rejects.toMatchInlineSnapshot(
Expand Down Expand Up @@ -530,13 +527,62 @@ describe('migrate-converged-pkg generator', () => {
expect(tree.exists(`${projectConfig.root}/config/api-extractor.local.json`)).toBeTruthy();
});
});

describe(`nx workspace updates`, () => {
it(`should set project sourceRoot and add tags to nx.json`, async () => {
let projectConfig = readProjectConfiguration(tree, options.name);

expect(projectConfig.sourceRoot).toBe(undefined);
expect(projectConfig.tags).toBe(undefined);
await generator(tree, options);

projectConfig = readProjectConfiguration(tree, options.name);

expect(projectConfig.sourceRoot).toBe(`${projectConfig.root}/src`);
expect(projectConfig.tags).toEqual(['vNext']);
});
});

describe(`--stats`, () => {
beforeEach(() => {
setupDummyPackage(tree, { name: '@proj/react-foo', version: '9.0.22' });
setupDummyPackage(tree, { name: '@proj/react-bar', version: '9.0.31' });
setupDummyPackage(tree, { name: '@proj/react-old', version: '8.1.12' });
setupDummyPackage(tree, { name: '@proj/react-older', version: '8.9.12' });
});

it(`should print project names and count of how many have been migrated`, async () => {
const loggerInfoSpy = jest.spyOn(logger, 'info');

await generator(tree, { stats: true });

expect(loggerInfoSpy.mock.calls[2][0]).toEqual('Migrated (0):');
expect(loggerInfoSpy.mock.calls[3][0]).toEqual('');
expect(loggerInfoSpy.mock.calls[5][0]).toEqual(`Not migrated (3):`);
expect(loggerInfoSpy.mock.calls[6][0]).toEqual(
expect.stringContaining(stripIndents`
- @proj/react-dummy
- @proj/react-foo
- @proj/react-bar
`),
);

loggerInfoSpy.mockClear();

await generator(tree, options);
await generator(tree, { stats: true });

expect(loggerInfoSpy.mock.calls[2][0]).toEqual('Migrated (1):');
expect(loggerInfoSpy.mock.calls[5][0]).toEqual(`Not migrated (2):`);
});
});
});

// ==== helpers ====

function setupDummyPackage(
tree: Tree,
options: MigrateConvergedPkgGeneratorSchema & Partial<{ version: string; dependencies: Record<string, string> }>,
options: AssertedSchema & Partial<{ version: string; dependencies: Record<string, string> }>,
) {
const workspaceConfig = readWorkspaceConfiguration(tree);
const defaults = {
Expand Down
78 changes: 72 additions & 6 deletions tools/generators/migrate-converged-pkg/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
visitNotIgnoredFiles,
logger,
writeJson,
updateProjectConfiguration,
} from '@nrwl/devkit';
import { serializeJson } from '@nrwl/workspace';
import { updateJestConfig } from '@nrwl/jest/src/generators/jest-project/lib/update-jestconfig';
Expand All @@ -29,12 +30,25 @@ import { MigrateConvergedPkgGeneratorSchema } from './schema';
* 5. update npm scripts (setup docs task to run api-extractor for local changes verification) - #18403 ✅
*/

interface ProjectConfiguration extends ReturnType<typeof readProjectConfiguration> {}

interface AssertedSchema extends MigrateConvergedPkgGeneratorSchema {
name: string;
}

interface NormalizedSchema extends ReturnType<typeof normalizeOptions> {}

type UserLog = Array<{ type: keyof typeof logger; message: string }>;

export default async function (tree: Tree, schema: MigrateConvergedPkgGeneratorSchema) {
const userLog: UserLog = [];

if (schema.stats) {
printStats(tree, schema);
// eslint-disable-next-line @typescript-eslint/no-empty-function
return () => {};
}

validateUserInput(tree, schema);

const options = normalizeOptions(tree, schema);
Expand All @@ -58,6 +72,8 @@ export default async function (tree: Tree, schema: MigrateConvergedPkgGeneratorS
updateNpmScripts(tree, options);
updateApiExtractorForLocalBuilds(tree, options);

updateNxWorkspace(tree, options);
Copy link
Member

Choose a reason for hiding this comment

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

What about the packages that are currently already migrated ? should those tags be added in this PR ?

Copy link
Contributor Author

@Hotell Hotell Jun 8, 2021

Choose a reason for hiding this comment

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

you can run it again on those


formatFiles(tree);

return () => {
Expand Down Expand Up @@ -146,7 +162,7 @@ const templates = {
},
};

function normalizeOptions(host: Tree, options: MigrateConvergedPkgGeneratorSchema) {
function normalizeOptions(host: Tree, options: AssertedSchema) {
const defaults = {};
const workspaceConfig = readWorkspaceConfiguration(host);
const projectConfig = readProjectConfiguration(host, options.name);
Expand Down Expand Up @@ -177,21 +193,71 @@ function normalizeOptions(host: Tree, options: MigrateConvergedPkgGeneratorSchem
};
}

function validateUserInput(tree: Tree, options: MigrateConvergedPkgGeneratorSchema) {
function validateUserInput(tree: Tree, options: MigrateConvergedPkgGeneratorSchema): asserts options is AssertedSchema {
if (!options.name) {
throw new Error(`--name cannot be empty. Please provide name of the package.`);
}

const projectConfig = readProjectConfiguration(tree, options.name);
const packageJson = readJson<PackageJson>(tree, joinPathFragments(projectConfig.root, 'package.json'));

const isPackageConverged = packageJson.version.startsWith('9.');

if (!isPackageConverged) {
if (!isPackageConverged(tree, projectConfig)) {
throw new Error(
`${options.name} is not converged package. Make sure to run the migration on packages with version 9.x.x`,
);
}
}

function printStats(tree: Tree, options: MigrateConvergedPkgGeneratorSchema) {
const allProjects = getProjects(tree);
const stats = {
migrated: [] as Array<ProjectConfiguration & { projectName: string }>,
notMigrated: [] as Array<ProjectConfiguration & { projectName: string }>,
};

allProjects.forEach((project, projectName) => {
if (!isPackageConverged(tree, project)) {
return;
}

if (isProjectMigrated(project)) {
stats.migrated.push({ projectName, ...project });

return;
}
stats.notMigrated.push({ projectName, ...project });
});

logger.info('Convergence DX migration stats:');
logger.info('='.repeat(80));

logger.info(`Migrated (${stats.migrated.length}):`);
logger.info(stats.migrated.map(projectStat => `- ${projectStat.projectName}`).join('\n'));

logger.info('='.repeat(80));
logger.info(`Not migrated (${stats.notMigrated.length}):`);
logger.info(stats.notMigrated.map(projectStat => `- ${projectStat.projectName}`).join('\n'));

return tree;
}

function isPackageConverged(tree: Tree, project: ProjectConfiguration) {
const packageJson = readJson<PackageJson>(tree, joinPathFragments(project.root, 'package.json'));
return packageJson.version.startsWith('9.');
}

function isProjectMigrated<T extends ProjectConfiguration>(
project: T,
): project is T & Required<Pick<ProjectConfiguration, 'tags' | 'sourceRoot'>> {
// eslint-disable-next-line eqeqeq
return project.sourceRoot != null && Boolean(project.tags?.includes('vNext'));
Copy link
Member

Choose a reason for hiding this comment

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

will this tag still be useful after all packages are migrated ? then I imagine we'll go back to a v9 validation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they will, we can enforce lint rules based on those - restrict importing anything to vNext besides vNext for example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to provide more context: nx.json and workspace.json gonna be main source of truth for any kind of project config metadata. (while we are in hybrid mode for convergence and non convergence, this approach is the best I could think of for now)

}

function updateNxWorkspace(tree: Tree, options: NormalizedSchema) {
updateProjectConfiguration(tree, options.name, {
...options.projectConfig,
sourceRoot: joinPathFragments(options.projectConfig.root, 'src'),
tags: [...(options.projectConfig.tags ?? []), 'vNext'],
});

return tree;
}
Expand Down
6 changes: 5 additions & 1 deletion tools/generators/migrate-converged-pkg/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@
},
"x-prompt": "Which converged package would you like migrate to new DX? (ex: @fluentui/react-menu)",
"pattern": "^[a-zA-Z].*$"
},
"stats": {
"type": "boolean",
"description": "Get statistics for how many projects have been migrated"
}
},
"required": ["name"]
"required": []
}
6 changes: 5 additions & 1 deletion tools/generators/migrate-converged-pkg/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,9 @@ export interface MigrateConvergedPkgGeneratorSchema {
/**
* Library name
*/
name: string;
name?: string;
/**
* Get statistics for how many projects have been migrated
*/
stats?: boolean;
}