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

chore: update generator to use Griffel #21390

Merged
merged 3 commits into from
Jan 24, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "add @types/react-test-renderer to devDependencies",
"packageName": "@fluentui/react-provider",
"email": "[email protected]",
"dependentChangeType": "none"
}
1 change: 1 addition & 0 deletions packages/react-provider/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@types/enzyme-adapter-react-16": "1.0.3",
"@types/react": "16.9.42",
"@types/react-dom": "16.9.10",
"@types/react-test-renderer": "^16.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

function replaceVersionsFromReference(
referencePackages: string[],
newPackageJson: PackageJson,
answers: Answers,
): void {

The script used for create-package reads & uses versions of dependencies, if this dependency absent it fails:

image

Opened for better suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

can we hoist this dev dependency ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean to move it to root package.json?

Copy link
Contributor

Choose a reason for hiding this comment

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

we need to move all of these devDep things to single version policy, would be nice to implement this in separate PR

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm working on PR to hoist this to single version policy

Copy link
Contributor

Choose a reason for hiding this comment

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

"enzyme": "~3.10.0",
"enzyme-adapter-react-16": "^1.15.0",
"react": "16.8.6",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { isConformant as baseIsConformant } from '@fluentui/react-conformance';
import type { IsConformantOptions, TestObject } from '@fluentui/react-conformance';
import makeStylesTests from '@fluentui/react-conformance-make-styles';
import griffelTests from '@fluentui/react-conformance-griffel';

export function isConformant<TProps = {}>(
testInfo: Omit<IsConformantOptions<TProps>, 'componentPath'> & { componentPath?: string },
) {
const defaultOptions: Partial<IsConformantOptions<TProps>> = {
asPropHandlesRef: true,
componentPath: module!.parent!.filename.replace('.test', ''),
extraTests: makeStylesTests as TestObject<TProps>,
extraTests: griffelTests as TestObject<TProps>,
};

baseIsConformant(defaultOptions, testInfo);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { makeStyles, mergeClasses } from '@fluentui/react-make-styles';
import { makeStyles, mergeClasses } from '@griffel/react';
import type { {{componentName}}State } from './{{componentName}}.types';

/**
Expand Down
3 changes: 2 additions & 1 deletion scripts/create-package/plop-templates-react/.babelrc.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"plugins": ["module:@fluentui/babel-make-styles", "annotate-pure-calls", "@babel/transform-react-pure-annotations"]
"presets": ["@griffel"],
"plugins": ["annotate-pure-calls", "@babel/transform-react-pure-annotations"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const path = require('path');

const config = createConfig({
setupFiles: [path.resolve(path.join(__dirname, 'config', 'tests.js'))],
snapshotSerializers: ['@fluentui/jest-serializer-make-styles'],
snapshotSerializers: ['@griffel/jest-serializer'],
});

module.exports = config;
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@
"update-snapshots": "just-scripts jest -u"
},
"devDependencies": {
"@fluentui/babel-make-styles": "",
"@fluentui/eslint-plugin": "",
"@fluentui/jest-serializer-make-styles": "",
Comment on lines -27 to -29
Copy link
Member Author

Choose a reason for hiding this comment

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

@fluentui/babel-make-styles & @fluentui/jest-serializer-make-styles are defined on root package.json as now they are third party devDependencies.

"@fluentui/react-conformance": "",
"@fluentui/react-conformance-make-styles": "",
"@fluentui/react-conformance-griffel": "",
"@fluentui/scripts": "",
"@types/enzyme": "",
"@types/enzyme-adapter-react-16": "",
Expand All @@ -42,9 +40,9 @@
"react-test-renderer": ""
},
"dependencies": {
"@fluentui/react-make-styles": "",
"@fluentui/react-theme": "",
"@fluentui/react-utilities": "",
"@griffel/react": "",
"tslib": ""
},
"peerDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion scripts/create-package/plopfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const v8ReferencePackages = {
node: ['codemods'],
};
const convergedReferencePackages = {
react: ['react-menu'],
react: ['react-provider'],
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently only @fluentui/react-provider is using Griffel packages (#21360). As this script pickups versions from a package.json I switched it to use different package as a source.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh didnt know we have thing like this, nx migration is run after the plop anyways, so it should be always up to date to latest setup

node: ['babel-make-styles'],
};

Expand Down
68 changes: 17 additions & 51 deletions tools/generators/migrate-converged-pkg/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ describe('migrate-converged-pkg generator', () => {
expect(rootTsConfig.compilerOptions.paths).toEqual(
expect.objectContaining({
'@proj/react-dummy': ['packages/react-dummy/src/index.ts'],
'@proj/react-make-styles': ['packages/react-make-styles/src/index.ts'],
Copy link
Member Author

Choose a reason for hiding this comment

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

This package will be removed soon.

'@proj/react-theme': ['packages/react-theme/src/index.ts'],
'@proj/react-utilities': ['packages/react-utilities/src/index.ts'],
}),
Expand Down Expand Up @@ -383,7 +382,7 @@ describe('migrate-converged-pkg generator', () => {

const config = createConfig({
setupFiles: [path.resolve(path.join(__dirname, 'config', 'tests.js'))],
snapshotSerializers: ['@fluentui/jest-serializer-make-styles'],
snapshotSerializers: ['@griffel/jest-serializer'],
});

module.exports = config;"
Expand Down Expand Up @@ -413,16 +412,15 @@ describe('migrate-converged-pkg generator', () => {
},
coverageDirectory: './coverage',
setupFilesAfterEnv: ['./config/tests.js'],
snapshotSerializers: ['@fluentui/jest-serializer-make-styles'],
snapshotSerializers: ['@griffel/jest-serializer'],
};"
`);
});

it(`should add 'snapshotSerializers' to jest.config.js only when needed`, async () => {
const projectConfig = readProjectConfiguration(tree, options.name);
function removePkgDependenciesThatTriggerSnapshotSerializersAddition() {
const workspaceConfig = readWorkspaceConfiguration(tree);
const packagesThatTriggerAddingSnapshots = [`@${workspaceConfig.npmScope}/react-make-styles`];
const packagesThatTriggerAddingSnapshots = ['@griffel/react'];

updateJson(tree, `${projectConfig.root}/package.json`, (json: PackageJson) => {
packagesThatTriggerAddingSnapshots.forEach(pkgName => {
Expand Down Expand Up @@ -958,94 +956,61 @@ describe('migrate-converged-pkg generator', () => {
}

it(`should setup .babelrc.json`, async () => {
const babelMakeStylesPkg = getScopedPkgName(tree, 'babel-make-styles');
const projectConfig = readProjectConfiguration(tree, options.name);

let packageJson = getPackageJson(projectConfig);
let devDeps = packageJson.devDependencies || {};
expect(devDeps[babelMakeStylesPkg]).toBe(undefined);

await generator(tree, options);

let babelConfig = getBabelConfig(projectConfig);

expect(babelConfig).toEqual({
plugins: [
'module:@fluentui/babel-make-styles',
'annotate-pure-calls',
'@babel/transform-react-pure-annotations',
],
presets: ['@griffel'],
plugins: ['annotate-pure-calls', '@babel/transform-react-pure-annotations'],
});

tree.delete(`${projectConfig.root}/.babelrc.json`);

await generator(tree, options);

babelConfig = getBabelConfig(projectConfig);
packageJson = getPackageJson(projectConfig);
devDeps = packageJson.devDependencies || {};

expect(babelConfig).toEqual({
plugins: [
'module:@fluentui/babel-make-styles',
'annotate-pure-calls',
'@babel/transform-react-pure-annotations',
],
presets: ['@griffel'],
plugins: ['annotate-pure-calls', '@babel/transform-react-pure-annotations'],
});
expect(devDeps[babelMakeStylesPkg]).toBe('9.0.0-alpha.0');
Copy link
Member Author

@layershifter layershifter Jan 24, 2022

Choose a reason for hiding this comment

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

We don't need to have Babel plugin in devDependencies of the package, that's why tests were trimmed a lot 🐱

});

it(`should add @fluentui/babel-make-styles plugin only if needed`, async () => {
it(`should add @griffel/babel-preset only if needed`, async () => {
let projectConfig = readProjectConfiguration(tree, options.name);
const babelMakeStylesPkg = getScopedPkgName(tree, 'babel-make-styles');

updateJson(tree, `${projectConfig.root}/package.json`, (json: PackageJson) => {
if (json.dependencies) {
delete json.dependencies[getScopedPkgName(tree, 'react-make-styles')];
delete json.dependencies[getScopedPkgName(tree, 'make-styles')];
delete json.dependencies['@griffel/react'];
}

json.devDependencies = json.devDependencies || {};
json.devDependencies[babelMakeStylesPkg] = '^9.0.0-alpha.0';

return json;
});

let babelConfig = getBabelConfig(projectConfig);
let packageJson = getPackageJson(projectConfig);
let devDeps = packageJson.devDependencies || {};

expect(babelConfig).toEqual({
plugins: [
'module:@fluentui/babel-make-styles',
'annotate-pure-calls',
'@babel/transform-react-pure-annotations',
],
presets: ['@griffel'],
plugins: ['annotate-pure-calls', '@babel/transform-react-pure-annotations'],
});
expect(devDeps[babelMakeStylesPkg]).toBe('^9.0.0-alpha.0');

await generator(tree, options);

babelConfig = getBabelConfig(projectConfig);
packageJson = getPackageJson(projectConfig);
devDeps = packageJson.devDependencies || {};

expect(babelConfig).toEqual({
presets: [],
plugins: ['annotate-pure-calls', '@babel/transform-react-pure-annotations'],
});
expect(devDeps[babelMakeStylesPkg]).toBe(undefined);

projectConfig = readProjectConfiguration(tree, '@proj/babel-make-styles');
await generator(tree, { name: '@proj/babel-make-styles' });

babelConfig = getBabelConfig(projectConfig);
packageJson = getPackageJson(projectConfig);
devDeps = packageJson.devDependencies || {};

expect(babelConfig).toEqual({
presets: [],
plugins: ['annotate-pure-calls', '@babel/transform-react-pure-annotations'],
});
expect(devDeps[babelMakeStylesPkg]).toBe(undefined);
});
});

Expand Down Expand Up @@ -1219,14 +1184,15 @@ function setupDummyPackage(
const defaults = {
version: '9.0.0-alpha.40',
dependencies: {
[`@${workspaceConfig.npmScope}/react-make-styles`]: '^9.0.0-alpha.38',
[`@griffel/react`]: '^9.0.0-alpha.38',
[`@${workspaceConfig.npmScope}/react-theme`]: '^9.0.0-alpha.13',
[`@${workspaceConfig.npmScope}/react-utilities`]: '^9.0.0-alpha.25',
tslib: '^2.1.0',
someThirdPartyDep: '^11.1.2',
},
babelConfig: {
plugins: ['module:@fluentui/babel-make-styles', 'annotate-pure-calls', '@babel/transform-react-pure-annotations'],
presets: ['@griffel'],
plugins: ['annotate-pure-calls', '@babel/transform-react-pure-annotations'],
},
tsConfig: { compilerOptions: { baseUrl: '.', typeRoots: ['../../node_modules/@types', '../../typings'] } },
};
Expand Down Expand Up @@ -1265,7 +1231,7 @@ function setupDummyPackage(

const config = createConfig({
setupFiles: [path.resolve(path.join(__dirname, 'config', 'tests.js'))],
snapshotSerializers: ['@fluentui/jest-serializer-make-styles'],
snapshotSerializers: ['@griffel/jest-serializer'],
});

module.exports = config;
Expand Down
30 changes: 8 additions & 22 deletions tools/generators/migrate-converged-pkg/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@ const templates = {
},
};
},
babelConfig: (options: { extraPlugins: Array<string> }) => {
babelConfig: (options: { extraPresets: Array<string> }) => {
return {
plugins: [...options.extraPlugins, 'annotate-pure-calls', '@babel/transform-react-pure-annotations'],
presets: [...options.extraPresets],
plugins: ['annotate-pure-calls', '@babel/transform-react-pure-annotations'],
};
},
jestSetup: stripIndents`
Expand All @@ -223,7 +224,7 @@ const templates = {
},
coverageDirectory: './coverage',
setupFilesAfterEnv: ['${options.testSetupFilePath}'],
${options.addSnapshotSerializers ? `snapshotSerializers: ['@fluentui/jest-serializer-make-styles'],` : ''}
${options.addSnapshotSerializers ? `snapshotSerializers: ['@griffel/jest-serializer'],` : ''}
};
`,
storybook: {
Expand Down Expand Up @@ -733,7 +734,7 @@ function shouldSetupE2E(tree: Tree, options: NormalizedSchema) {

function updateLocalJestConfig(tree: Tree, options: NormalizedSchema) {
const jestSetupFilePath = options.paths.jestSetupFile;
const packagesThatTriggerAddingSnapshots = [`@${options.workspaceConfig.npmScope}/react-make-styles`];
const packagesThatTriggerAddingSnapshots = [`@griffel/react`];

const packageJson = readJson<PackageJson>(tree, options.paths.packageJson);
packageJson.dependencies = packageJson.dependencies ?? {};
Expand Down Expand Up @@ -834,28 +835,13 @@ function updatedBaseTsConfig(tree: Tree, options: NormalizedSchema) {
}

function setupBabel(tree: Tree, options: NormalizedSchema) {
const currentProjectNpmScope = `@${options.workspaceConfig.npmScope}`;
const pkgJson = readJson<PackageJson>(tree, options.paths.packageJson);
pkgJson.dependencies = pkgJson.dependencies || {};
pkgJson.devDependencies = pkgJson.devDependencies || {};

const shouldAddMakeStylesPlugin =
!options.name.includes('make-styles') &&
(pkgJson.dependencies[`${currentProjectNpmScope}/react-make-styles`] ||
pkgJson.dependencies[`${currentProjectNpmScope}/make-styles`]);

const extraPlugins = shouldAddMakeStylesPlugin ? ['module:@fluentui/babel-make-styles'] : [];

const config = templates.babelConfig({ extraPlugins });

const babelMakeStylesProjectName = `${currentProjectNpmScope}/babel-make-styles`;
if (shouldAddMakeStylesPlugin) {
const babelMakeStylesProject = getProjectConfig(tree, { packageName: babelMakeStylesProjectName });
const babelMakeStylesPkgJson: PackageJson = readJson(tree, babelMakeStylesProject.paths.packageJson);
pkgJson.devDependencies[babelMakeStylesProjectName] = `${babelMakeStylesPkgJson.version}`;
} else {
delete pkgJson.devDependencies[babelMakeStylesProjectName];
}
const shouldAddGriffelPreset = pkgJson.dependencies['@griffel/react'];
const extraPresets = shouldAddGriffelPreset ? ['@griffel'] : [];
const config = templates.babelConfig({ extraPresets });
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no need to add Babel plugin to devDependencies, this simplifies this part a lot.

Copy link
Contributor

Choose a reason for hiding this comment

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

me gusta


tree.write(options.paths.babelConfig, serializeJson(config));
writeJson(tree, options.paths.packageJson, pkgJson);
Expand Down