From f5f269b4fb23de4f5ed00b2397945e0b00570754 Mon Sep 17 00:00:00 2001 From: Miroslav Jonas Date: Wed, 19 Apr 2023 11:53:49 +0200 Subject: [PATCH] fix(core): enforce fixed versions only for apps with createPackageJson --- .../package-json/update-package-json.spec.ts | 4 +- .../package-json/create-package-json.spec.ts | 216 +++++++++++++++++- .../js/package-json/create-package-json.ts | 89 +++++--- 3 files changed, 278 insertions(+), 31 deletions(-) diff --git a/packages/js/src/utils/package-json/update-package-json.spec.ts b/packages/js/src/utils/package-json/update-package-json.spec.ts index 9209b21eb3d5c..0adb33c728c3f 100644 --- a/packages/js/src/utils/package-json/update-package-json.spec.ts +++ b/packages/js/src/utils/package-json/update-package-json.spec.ts @@ -440,8 +440,8 @@ describe('updatePackageJson', () => { expect(distPackageJson).toMatchInlineSnapshot(` { "dependencies": { - "external1": "1.0.0", - "external2": "4.5.6", + "external1": "~1.0.0", + "external2": "^4.0.0", }, "main": "./main.js", "name": "@org/lib1", diff --git a/packages/nx/src/plugins/js/package-json/create-package-json.spec.ts b/packages/nx/src/plugins/js/package-json/create-package-json.spec.ts index 5a84a65557375..3fd7406f27b87 100644 --- a/packages/nx/src/plugins/js/package-json/create-package-json.spec.ts +++ b/packages/nx/src/plugins/js/package-json/create-package-json.spec.ts @@ -1,11 +1,15 @@ import * as fs from 'fs'; import * as configModule from '../../../config/configuration'; -import { DependencyType } from '../../../config/project-graph'; +import { DependencyType, ProjectGraph } from '../../../config/project-graph'; import * as hashModule from '../../../hasher/hasher'; import { createPackageJson } from './create-package-json'; import * as fileutilsModule from '../../../utils/fileutils'; +jest.mock('../../../utils/workspace-root', () => ({ + workspaceRoot: '/root', +})); + describe('createPackageJson', () => { it('should add additional dependencies', () => { jest.spyOn(fs, 'existsSync').mockReturnValue(false); @@ -499,4 +503,214 @@ describe('createPackageJson', () => { ); expect(filterUsingGlobPatternsSpy).toHaveBeenCalledTimes(4); }); + + describe('parsing "package.json" versions', () => { + const appDependencies = [ + { source: 'app1', target: 'npm:@nx/devkit', type: 'static' }, + { source: 'app1', target: 'npm:typescript', type: 'static' }, + ]; + + const libDependencies = [ + { source: 'lib1', target: 'npm:@nx/devkit', type: 'static' }, + { source: 'lib1', target: 'npm:tslib', type: 'static' }, + { source: 'lib1', target: 'npm:typescript', type: 'static' }, + ]; + + const graph: ProjectGraph = { + nodes: { + app1: { + type: 'app', + name: 'app1', + data: { + files: [ + { + file: '/root/apps/app1/src/main.ts', + hash: '', + dependencies: appDependencies, + }, + ], + targets: {}, + root: '/root/apps/app1', + }, + }, + lib1: { + type: 'lib', + name: 'lib1', + data: { + files: [ + { + file: '/root/libs/lib1/index.ts', + hash: '', + dependencies: libDependencies, + }, + ], + targets: {}, + root: '/root/libs/lib1', + }, + }, + }, + externalNodes: { + 'npm:@nx/devkit': { + type: 'npm', + name: 'npm:@nx/devkit', + data: { version: '16.0.0', hash: '', packageName: '@nx/devkit' }, + }, + 'npm:nx': { + type: 'npm', + name: 'npm:nx', + data: { version: '16.0.0', hash: '', packageName: 'nx' }, + }, + 'npm:tslib': { + type: 'npm', + name: 'npm:tslib', + data: { version: '2.4.4', hash: '', packageName: 'tslib' }, + }, + 'npm:typescript': { + type: 'npm', + name: 'npm:typescript', + data: { version: '4.9.5', hash: '', packageName: 'typescript' }, + }, + }, + dependencies: { + app1: appDependencies, + lib1: libDependencies, + }, + }; + + const rootPackageJson = () => ({ + dependencies: { + '@nx/devkit': '~16.0.0', + nx: '> 14', + typescript: '^4.8.2', + tslib: '~2.4.0', + }, + }); + + const projectPackageJson = () => ({ + name: 'other-name', + version: '1.2.3', + dependencies: { + typescript: '^4.8.4', + random: '1.0.0', + }, + }); + + const spies = []; + + afterEach(() => { + while (spies.length > 0) { + spies.pop().mockRestore(); + } + }); + + it('should use fixed versions when creating package json for apps', () => { + spies.push( + jest + .spyOn(fileutilsModule, 'readJsonFile') + .mockImplementation((path) => { + if (path === '/root/package.json') { + return rootPackageJson(); + } + }) + ); + + expect(createPackageJson('app1', graph)).toEqual({ + dependencies: { + '@nx/devkit': '16.0.0', + nx: '16.0.0', + typescript: '4.9.5', + }, + name: 'app1', + version: '0.0.1', + }); + }); + + it('should override fixed versions with local ranges when creating package json for apps', () => { + spies.push( + jest.spyOn(fs, 'existsSync').mockImplementation((path) => { + if (path === '/root/apps/app1/package.json') { + return true; + } + }) + ); + spies.push( + jest + .spyOn(fileutilsModule, 'readJsonFile') + .mockImplementation((path) => { + if (path === '/root/package.json') { + return rootPackageJson(); + } + if (path === '/root/apps/app1/package.json') { + return projectPackageJson(); + } + }) + ); + + expect(createPackageJson('app1', graph)).toEqual({ + dependencies: { + '@nx/devkit': '16.0.0', + nx: '16.0.0', + random: '1.0.0', + typescript: '^4.8.4', + }, + name: 'other-name', + version: '1.2.3', + }); + }); + + it('should use range versions when creating package json for libs', () => { + spies.push( + jest + .spyOn(fileutilsModule, 'readJsonFile') + .mockImplementation((path) => { + if (path === '/root/package.json') { + return rootPackageJson(); + } + }) + ); + + expect(createPackageJson('lib1', graph)).toEqual({ + dependencies: { + '@nx/devkit': '~16.0.0', + tslib: '~2.4.0', + typescript: '^4.8.2', + }, + name: 'lib1', + version: '0.0.1', + }); + }); + + it('should override range versions with local ranges when creating package json for libs', () => { + spies.push( + jest.spyOn(fs, 'existsSync').mockImplementation((path) => { + if (path === '/root/libs/lib1/package.json') { + return true; + } + }) + ); + spies.push( + jest + .spyOn(fileutilsModule, 'readJsonFile') + .mockImplementation((path) => { + if (path === '/root/package.json') { + return rootPackageJson(); + } + if (path === '/root/libs/lib1/package.json') { + return projectPackageJson(); + } + }) + ); + + expect(createPackageJson('lib1', graph)).toEqual({ + dependencies: { + '@nx/devkit': '~16.0.0', + random: '1.0.0', + tslib: '~2.4.0', + typescript: '^4.8.4', + }, + name: 'other-name', + version: '1.2.3', + }); + }); + }); }); diff --git a/packages/nx/src/plugins/js/package-json/create-package-json.ts b/packages/nx/src/plugins/js/package-json/create-package-json.ts index 298a04970a3c4..f56adef38f4fb 100644 --- a/packages/nx/src/plugins/js/package-json/create-package-json.ts +++ b/packages/nx/src/plugins/js/package-json/create-package-json.ts @@ -36,6 +36,7 @@ export function createPackageJson( } = {} ): PackageJson { const projectNode = graph.nodes[projectName]; + const isLibrary = projectNode.type === 'lib'; const { selfInputs, dependencyInputs } = options.target ? getTargetInputs(readNxJson(), projectNode, options.target) @@ -91,6 +92,18 @@ export function createPackageJson( } catch (e) {} } + const getVersion = ( + packageName, + version, + section: 'devDependencies' | 'dependencies' + ) => { + return ( + packageJson[section][packageName] || + (isLibrary && rootPackageJson[section]?.[packageName]) || + version + ); + }; + const rootPackageJson = readJsonFile( `${options.root || workspaceRoot}/package.json` ); @@ -103,44 +116,64 @@ export function createPackageJson( // don't store dev dependencies for production if (!options.isProduction) { packageJson.devDependencies ??= {}; - packageJson.devDependencies[packageName] = version; + packageJson.devDependencies[packageName] = getVersion( + packageName, + version, + 'devDependencies' + ); } } else { if (!packageJson.peerDependencies?.[packageName]) { packageJson.dependencies ??= {}; - packageJson.dependencies[packageName] = version; + packageJson.dependencies[packageName] = getVersion( + packageName, + version, + 'dependencies' + ); } } }); - Object.entries(npmDeps.peerDependencies).forEach(([packageName, version]) => { - if (!packageJson.peerDependencies?.[packageName]) { - if (rootPackageJson.dependencies?.[packageName]) { - packageJson.dependencies ??= {}; - packageJson.dependencies[packageName] = version; - return; - } + if (!isLibrary) { + Object.entries(npmDeps.peerDependencies).forEach( + ([packageName, version]) => { + if (!packageJson.peerDependencies?.[packageName]) { + if (rootPackageJson.dependencies?.[packageName]) { + packageJson.dependencies ??= {}; + packageJson.dependencies[packageName] = getVersion( + packageName, + version, + 'dependencies' + ); + return; + } - const isOptionalPeer = - npmDeps.peerDependenciesMeta[packageName]?.optional; - if (!isOptionalPeer) { - if ( - !options.isProduction || - rootPackageJson.dependencies?.[packageName] - ) { - packageJson.peerDependencies ??= {}; - packageJson.peerDependencies[packageName] = version; + const isOptionalPeer = + npmDeps.peerDependenciesMeta[packageName]?.optional; + if (!isOptionalPeer) { + if ( + !options.isProduction || + rootPackageJson.dependencies?.[packageName] + ) { + packageJson.peerDependencies ??= {}; + packageJson.peerDependencies[packageName] = getVersion( + packageName, + version, + 'dependencies' + ); + } + } else if (!options.isProduction) { + // add peer optional dependencies if not in production + packageJson.peerDependencies ??= {}; + packageJson.peerDependencies[packageName] = version; + packageJson.peerDependenciesMeta ??= {}; + packageJson.peerDependenciesMeta[packageName] = { + optional: true, + }; + } } - } else if (!options.isProduction) { - // add peer optional dependencies if not in production - packageJson.peerDependencies ??= {}; - packageJson.peerDependencies[packageName] = version; - packageJson.peerDependenciesMeta ??= {}; - packageJson.peerDependenciesMeta[packageName] = { - optional: true, - }; } - } - }); + ); + } packageJson.devDependencies &&= sortObjectByKeys(packageJson.devDependencies); packageJson.dependencies &&= sortObjectByKeys(packageJson.dependencies);