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

fix: the tsconfig spec generated for library contains several issues #2584

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Expand Up @@ -297,7 +297,7 @@ export function setPackagerManagerConfig(options: PackageManagerConfig, execAppO
execFileSync('yarn', ['config', 'set', 'enableGlobalCache', 'true'], execOptions);
execFileSync('yarn', ['config', 'set', 'globalFolder', options.globalFolderPath], execOptions);
}
execFileSync('yarn', ['config', 'set', 'nodeLinker', 'pnp'], execOptions);
execFileSync('yarn', ['config', 'set', 'nodeLinker', 'node-modules'], execOptions);
Copy link
Contributor

Choose a reason for hiding this comment

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

The purpose of the e2e with yarn is to test the default config of yarn which is PNP.
Not sure we should change it :S.
Why did you need to?

cpaulve-1A marked this conversation as resolved.
Show resolved Hide resolved
execFileSync('yarn', ['config', 'set', 'npmScopes.ama-sdk.npmRegistryServer', options.registry], execOptions);
execFileSync('yarn', ['config', 'set', 'npmScopes.ama-terasu.npmRegistryServer', options.registry], execOptions);
execFileSync('yarn', ['config', 'set', 'npmScopes.o3r.npmRegistryServer', options.registry], execOptions);
Expand Down
1 change: 1 addition & 0 deletions packages/@o3r/workspace/schematics/index.it.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ describe('new otter workspace', () => {
expect(existsSync(path.join(workspacePath, 'project'))).toBe(false);
generatedLibFiles.forEach((file) => expect(existsSync(path.join(inLibraryPath, file))).toBe(true));
expect(() => packageManagerRunOnProject(libName, true, { script: 'build' }, execAppOptions)).not.toThrow();
expect(() => packageManagerExec({ script: 'ng', args: ['test', '--watch=false', '--browsers=ChromeHeadless'] }, execAppOptions)).not.toThrow();
});

test('should generate a monorepo setup', async () => {
Expand Down
41 changes: 37 additions & 4 deletions packages/@o3r/workspace/schematics/library/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ jest.mock('@angular-devkit/schematics', () => {
});

const collectionPath = path.join(__dirname, '..', '..', 'collection.json');
const angularJsonFile = `{
"version": 1,
"projects": {
"my-new-module": {
"projectType": "library",
"root": "packages-test/my-new-module"
}
}
}`;

describe('New module generator', () => {
let initialTree: Tree;
Expand All @@ -36,7 +45,7 @@ describe('New module generator', () => {
});

it('should generate the minimum mandatory files', async () => {
initialTree.create('angular.json', '{"version": 1, "projects": {} }');
initialTree.create('angular.json', angularJsonFile);
initialTree.create('package.json', '{ "version": "0.0.0-test" }');
initialTree.create('/packages-test/my-new-module/package.json', '{ "version": "0.0.0-test" }');
initialTree.create('/packages-test/my-new-module/ng-package.json', '{ }');
Expand All @@ -58,11 +67,36 @@ describe('New module generator', () => {
expect(tree.exists('/packages-test/my-new-module/project.json')).toBe(false);
expect(JSON.parse(tree.readContent('/tsconfig.base.json')).compilerOptions.paths['@my/new-module']).toContain('packages-test/my-new-module/src/public-api');
expect(JSON.parse(tree.readContent('/tsconfig.build.json')).compilerOptions.paths['@my/new-module'][0]).toBe('packages-test/my-new-module/dist');
expect(tree.exists('/packages-test/my-new-module/testing/setup-jest.ts')).toBe(false);
expect(tree.exists('/packages-test/my-new-module/jest.config.js')).toBe(false);
expect(tree.files.length).toBeGreaterThanOrEqual(9);
});

// eslint-disable-next-line jest/no-disabled-tests -- TODO: Should be re-enable when the following issue #2066 is fixed
describe.skip('in NX monorepo', () => {
it('should generate an project with jest files', async () => {
initialTree.create('angular.json', angularJsonFile);
initialTree.create('package.json', '{ "version": "0.0.0-test" }');
initialTree.create('/packages-test/my-new-module/package.json', '{ "version": "0.0.0-test" }');
initialTree.create('/packages-test/my-new-module/ng-package.json', '{ }');
const runner = new SchematicTestRunner('schematics', collectionPath);
const angularPackageJson = require.resolve('@schematics/angular/package.json');
const o3rCorePackageJson = require.resolve('@o3r/core/package.json');
runner.registerCollection('@o3r/core', path.resolve(path.dirname(o3rCorePackageJson), require(o3rCorePackageJson).schematics));
runner.registerCollection('@schematics/angular', path.resolve(path.dirname(angularPackageJson), require(angularPackageJson).schematics));
jest.spyOn(require('@angular-devkit/schematics'), 'externalSchematic');
const tree = await runner.runSchematic('library', {
path: 'packages-test',
name: '@my/new-module',
skipLinter: true,
skipInstall: true,
testingFramework: 'jest'

}, initialTree);
expect(tree.exists('/packages-test/my-new-module/testing/setup-jest.ts')).toBe(true);
expect(tree.exists('/packages-test/my-new-module/jest.config.js')).toBe(true);
expect(JSON.parse(tree.readContent('/packages-test/my-new-module/package.json')).scripts.test).toContain('jest');
});

describe('in NX monorepo', () => {
it('should generate Nx project.json with given name', async () => {
initialTree.create('nx.json', '{"workspaceLayout": { "libsDir": "packages-test" } }');
initialTree.create('angular.json', '{"version": 1, "projects": {} }');
Expand All @@ -77,7 +111,6 @@ describe('New module generator', () => {
runner.registerCollection('@schematics/angular', path.resolve(path.dirname(angularPackageJson), require(angularPackageJson).schematics));
runner.registerCollection('@nx/workspace', path.resolve(path.dirname(nxWorkspacePackageJson), require(nxWorkspacePackageJson).generators));
const tree = await runner.runExternalSchematic('schematics', 'library', {
path: 'packages-test',
name: '@my/new-module',
projectName: 'test-module-name',
skipLinter: true
Expand Down
17 changes: 17 additions & 0 deletions packages/@o3r/workspace/schematics/library/rules/rules.ng.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import type {
NgGenerateModuleSchema,
} from '../schema';
import {
setUpJest,
updateNgPackagrFactory,
updatePackageDependenciesFactory,
} from './shared';
Expand Down Expand Up @@ -61,9 +62,25 @@ export function ngGenerateModule(options: NgGenerateModuleSchema & { targetPath:
renameTemplateFiles(),
move(options.targetPath)
]);
const templateJest = apply(url('./templates/jest'), [
template({
...options,
tsconfigBasePath: findConfigFileRelativePath(tree, ['tsconfig.base.json', 'tsconfig.json'], options.targetPath)
}),
renameTemplateFiles(),
move(options.targetPath)
]);
const packageJsonContent = tree.readText('/package.json');
const hasJestInstalled = options.testingFramework === 'jest' || packageJsonContent.match('jest');
cpaulve-1A marked this conversation as resolved.
Show resolved Hide resolved

return chain([
mergeWith(templateNg, MergeStrategy.Overwrite),
...hasJestInstalled
? [
mergeWith(templateJest, MergeStrategy.Overwrite),
setUpJest(options)
]
: [],
updatePackageDependenciesFactory(options.targetPath, otterVersion!, o3rCorePackageJson, options),
updateNgPackagrFactory(options.targetPath),
(t) => {
Expand Down
17 changes: 17 additions & 0 deletions packages/@o3r/workspace/schematics/library/rules/rules.nx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import type {
NgGenerateModuleSchema,
} from '../schema';
import {
setUpJest,
updateNgPackagrFactory,
updatePackageDependenciesFactory,
} from './shared';
Expand Down Expand Up @@ -118,12 +119,28 @@ export function nxGenerateModule(options: NgGenerateModuleSchema & { packageJson
renameTemplateFiles(),
move(targetPath)
]);
const templateJest = apply(url('./templates/jest'), [
template({
...options,
tsconfigBasePath: findConfigFileRelativePath(tree, ['tsconfig.base.json', 'tsconfig.json'], targetPath)
}),
renameTemplateFiles(),
move(targetPath)
]);
rules.push(mergeWith(templateNx, MergeStrategy.Overwrite));
const packageJsonContent = tree.readText('/package.json');
const hasJestInstalled = options.testingFramework === 'jest' || packageJsonContent.match('jest');
cpaulve-1A marked this conversation as resolved.
Show resolved Hide resolved

return chain([
...rules,
updatePackageDependenciesFactory(targetPath, otterVersion!, o3rCorePackageJson, options),
updateNgPackagrFactory(targetPath),
...hasJestInstalled
? [
mergeWith(templateJest, MergeStrategy.Overwrite),
setUpJest(options)
]
: [],
(t) => {
const packageJson = t.readJson(path.posix.join(targetPath, 'package.json')) as PackageJson;
packageJson.name = options.packageJsonName;
Expand Down
23 changes: 23 additions & 0 deletions packages/@o3r/workspace/schematics/library/rules/shared.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import * as path from 'node:path';
import type {
Rule,
SchematicContext,
Tree,
} from '@angular-devkit/schematics';
import {
getPackageManagerRunner,
Expand All @@ -13,6 +15,27 @@
NgGenerateModuleSchema,
} from '../schema';

/**
* Set jest files and script in the generated library.
* @param options
*/
export function setUpJest(options: NgGenerateModuleSchema) {
return (tree: Tree, context: SchematicContext) => {
const workspaceConfig = getWorkspaceConfig(tree);
const workspaceProject = (options.name && workspaceConfig?.projects?.[options.name]) || undefined;
if (!workspaceProject?.root) {
context.logger.error(`Failed to find a package json for ${options.name}`);
return;
}

Check warning on line 29 in packages/@o3r/workspace/schematics/library/rules/shared.ts

View check run for this annotation

Codecov / codecov/patch

packages/@o3r/workspace/schematics/library/rules/shared.ts#L28-L29

Added lines #L28 - L29 were not covered by tests
const packageJsonPath = path.join(workspaceProject.root, 'package.json');
const packageJsonContent = tree.readJson(packageJsonPath) as PackageJson;
packageJsonContent.scripts ||= {};
packageJsonContent.scripts.test ||= 'jest';
tree.overwrite(packageJsonPath, JSON.stringify(packageJsonContent, null, 2));
return tree;
};
}

/**
* Generate rule to update generated package.json file
* @param targetPath Path of the generated files
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add an it-test https://github.com/AmadeusITGroup/otter/blob/main/packages/%40o3r/workspace/schematics/index.it.spec.ts#L117 to make sure the test are working on the generated lib?

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,10 @@ export function addMonorepoManager(o3rWorkspacePackageJson: PackageJson & { gene
lint: 'lerna run lint'
};

const lernaJson: { $schema: string; version: string; npmClient?: string } = {
const lernaJson: { $schema: string; version: string; npmClient?: string; useNx?: boolean } = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove this change, wrongly commited

$schema: 'https://github.com/lerna/lerna/blob/main/packages/lerna/schemas/lerna-schema.json',
version: rootPackageJsonObject.version || '0.0.0-placeholder'
version: rootPackageJsonObject.version || '0.0.0-placeholder',
useNx: false
};
if (getPackageManager() === 'yarn') {
lernaJson.npmClient = 'yarn';
Expand Down
27 changes: 27 additions & 0 deletions tools/github-actions/new-version/packaged-action/LICENSE.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading