From b07d24ef420db4df5f7cbaf7d7df49041d1ca588 Mon Sep 17 00:00:00 2001 From: Jack Hsu Date: Tue, 18 Jul 2023 02:53:18 -0400 Subject: [PATCH] feat(js): create package.json with proper defaults (#18091) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Miroslav Jonaš --- e2e/esbuild/src/esbuild.test.ts | 36 ++-- e2e/js/src/js-bundling.test.ts | 33 ---- e2e/js/src/js-packaging.test.ts | 171 ++++++++++++++++++ e2e/js/src/js-swc.test.ts | 6 + e2e/js/src/js-tsc.test.ts | 12 ++ e2e/linter/src/linter.test.ts | 18 +- .../js/src/generators/library/library.spec.ts | 1 + packages/js/src/generators/library/library.ts | 88 ++++++++- 8 files changed, 299 insertions(+), 66 deletions(-) delete mode 100644 e2e/js/src/js-bundling.test.ts create mode 100644 e2e/js/src/js-packaging.test.ts diff --git a/e2e/esbuild/src/esbuild.test.ts b/e2e/esbuild/src/esbuild.test.ts index 8cb740adc0f94..5299da01e111d 100644 --- a/e2e/esbuild/src/esbuild.test.ts +++ b/e2e/esbuild/src/esbuild.test.ts @@ -42,28 +42,24 @@ describe('EsBuild Plugin', () => { name: `@proj/${myPkg}`, version: '0.0.1', type: 'commonjs', + main: './index.cjs', + dependencies: {}, }); // Build normally with package.json generation. runCLI(`build ${myPkg}`); - expect(runCommand(`node dist/libs/${myPkg}/index.js`)).toMatch(/Hello/); + expect(runCommand(`node dist/libs/${myPkg}/index.cjs`)).toMatch(/Hello/); // main field should be set correctly in package.json checkFilesExist(`dist/libs/${myPkg}/package.json`); expect(runCommand(`node dist/libs/${myPkg}`)).toMatch(/Hello/); - expect(runCommand(`node dist/libs/${myPkg}/index.js`)).toMatch(/Hello/); + expect(runCommand(`node dist/libs/${myPkg}/index.cjs`)).toMatch(/Hello/); // main field should be set correctly in package.json expect(readFile(`dist/libs/${myPkg}/assets/a.md`)).toMatch(/file a/); expect(readFile(`dist/libs/${myPkg}/assets/b.md`)).toMatch(/file b/); - /* CJS format is not used by default, but passing --format=esm,cjs generates with it. - */ - checkFilesDoNotExist(`dist/libs/${myPkg}/index.cjs`); - runCLI(`build ${myPkg} --format=esm,cjs`); - checkFilesExist(`dist/libs/${myPkg}/index.cjs`); - /* Metafile is not generated by default, but passing --metafile generates it. */ checkFilesDoNotExist(`dist/libs/${myPkg}/meta.json`); @@ -81,7 +77,7 @@ describe('EsBuild Plugin', () => { ); expect(() => runCLI(`build ${myPkg}`)).toThrow(); expect(() => runCLI(`build ${myPkg} --skipTypeCheck`)).not.toThrow(); - expect(runCommand(`node dist/libs/${myPkg}/index.js`)).toMatch(/Bye/); + expect(runCommand(`node dist/libs/${myPkg}/index.cjs`)).toMatch(/Bye/); // Reset file updateFile( `libs/${myPkg}/src/index.ts`, @@ -141,7 +137,7 @@ describe('EsBuild Plugin', () => { expect( readJson(`dist/libs/${parentLib}/package.json`).dependencies?.['dayjs'] ).not.toBeDefined(); - let runResult = runCommand(`node dist/libs/${parentLib}/index.js`); + let runResult = runCommand(`node dist/libs/${parentLib}/index.cjs`); expect(runResult).toMatch(/Hello world/); expect(runResult).toMatch(/Hello from child lib/); @@ -155,7 +151,7 @@ describe('EsBuild Plugin', () => { rambda: expect.any(String), lodash: expect.any(String), }); - runResult = runCommand(`node dist/libs/${parentLib}/index.js`); + runResult = runCommand(`node dist/libs/${parentLib}/index.cjs`); expect(runResult).toMatch(/Hello world/); expect(runResult).toMatch(/Hello from child lib/); }, 300_000); @@ -164,16 +160,18 @@ describe('EsBuild Plugin', () => { const myPkg = uniq('my-pkg'); runCLI(`generate @nx/js:lib ${myPkg} --bundler=esbuild`); updateFile(`libs/${myPkg}/src/lib/${myPkg}.ts`, `console.log('Hello');\n`); - updateFile(`libs/${myPkg}/src/index.ts`, `import './lib/${myPkg}.js';\n`); + updateFile(`libs/${myPkg}/src/index.ts`, `import './lib/${myPkg}.cjs';\n`); runCLI(`build ${myPkg} --bundle=false`); checkFilesExist( - `dist/libs/${myPkg}/lib/${myPkg}.js`, - `dist/libs/${myPkg}/index.js` + `dist/libs/${myPkg}/libs/${myPkg}/src/lib/${myPkg}.cjs`, + `dist/libs/${myPkg}/index.cjs` ); // Test files are excluded in tsconfig (e.g. tsconfig.lib.json) - checkFilesDoNotExist(`dist/libs/${myPkg}/lib/${myPkg}.spec.js`); + checkFilesDoNotExist( + `dist/libs/${myPkg}/libs/${myPkg}/src/lib/${myPkg}.spec.cjs` + ); // Can run package (package.json fields are correctly generated) expect(runCommand(`node dist/libs/${myPkg}`)).toMatch(/Hello/); }, 300_000); @@ -193,14 +191,14 @@ describe('EsBuild Plugin', () => { runCLI(`build ${myPkg}`); checkFilesExist( - `dist/libs/${myPkg}/index.js`, - `dist/libs/${myPkg}/extra.js` + `dist/libs/${myPkg}/index.cjs`, + `dist/libs/${myPkg}/extra.cjs` ); expect( - runCommand(`node dist/libs/${myPkg}/index.js`, { failOnError: true }) + runCommand(`node dist/libs/${myPkg}/index.cjs`, { failOnError: true }) ).toMatch(/main/); expect( - runCommand(`node dist/libs/${myPkg}/extra.js`, { failOnError: true }) + runCommand(`node dist/libs/${myPkg}/extra.cjs`, { failOnError: true }) ).toMatch(/extra/); }, 120_000); diff --git a/e2e/js/src/js-bundling.test.ts b/e2e/js/src/js-bundling.test.ts deleted file mode 100644 index d2ad326b6c202..0000000000000 --- a/e2e/js/src/js-bundling.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { - checkFilesExist, - cleanupProject, - newProject, - runCLI, - uniq, -} from '@nx/e2e/utils'; - -describe('bundling libs', () => { - let scope: string; - - beforeEach(() => { - scope = newProject(); - }); - - afterEach(() => cleanupProject()); - - it('should support esbuild and vite bundlers for building libs', () => { - const esbuildLib = uniq('esbuildlib'); - const viteLib = uniq('vitelib'); - - runCLI( - `generate @nx/js:lib ${esbuildLib} --bundler=esbuild --no-interactive` - ); - runCLI(`generate @nx/js:lib ${viteLib} --bundler=vite --no-interactive`); - - runCLI(`build ${esbuildLib}`); - runCLI(`build ${viteLib}`); - - checkFilesExist(`dist/libs/${esbuildLib}/index.js`); - checkFilesExist(`dist/libs/${viteLib}/index.js`); - }, 240_000); -}); diff --git a/e2e/js/src/js-packaging.test.ts b/e2e/js/src/js-packaging.test.ts new file mode 100644 index 0000000000000..a9367c4d5878c --- /dev/null +++ b/e2e/js/src/js-packaging.test.ts @@ -0,0 +1,171 @@ +import { + checkFilesExist, + cleanupProject, + newProject, + runCLI, + tmpProjPath, + runCommand, + createFile, + uniq, + getPackageManagerCommand, +} from '@nx/e2e/utils'; +import { join } from 'path'; +import { ensureDirSync } from 'fs-extra'; + +describe('bundling libs', () => { + let scope: string; + + beforeEach(() => { + scope = newProject(); + }); + + afterEach(() => cleanupProject()); + + it('should support esbuild, rollup, vite bundlers for building libs', () => { + const esbuildLib = uniq('esbuildlib'); + const viteLib = uniq('vitelib'); + const rollupLib = uniq('rolluplib'); + + runCLI( + `generate @nx/js:lib ${esbuildLib} --bundler=esbuild --no-interactive` + ); + runCLI(`generate @nx/js:lib ${viteLib} --bundler=vite --no-interactive`); + runCLI( + `generate @nx/js:lib ${rollupLib} --bundler=rollup --no-interactive` + ); + + runCLI(`build ${esbuildLib}`); + runCLI(`build ${viteLib}`); + runCLI(`build ${rollupLib}`); + + const pmc = getPackageManagerCommand(); + let output: string; + + // Make sure outputs in commonjs project + createFile( + 'test-cjs/package.json', + JSON.stringify( + { + name: 'test-cjs', + private: true, + type: 'commonjs', + dependencies: { + [`@proj/${esbuildLib}`]: `file:../dist/libs/${esbuildLib}`, + [`@proj/${viteLib}`]: `file:../dist/libs/${viteLib}`, + [`@proj/${rollupLib}`]: `file:../dist/libs/${rollupLib}`, + }, + }, + null, + 2 + ) + ); + createFile( + 'test-cjs/index.js', + ` + const { ${esbuildLib} } = require('@proj/${esbuildLib}'); + const { ${viteLib} } = require('@proj/${viteLib}'); + const { ${rollupLib} } = require('@proj/${rollupLib}'); + console.log(${esbuildLib}()); + console.log(${viteLib}()); + console.log(${rollupLib}()); + ` + ); + runCommand(pmc.install, { + cwd: join(tmpProjPath(), 'test-cjs'), + }); + output = runCommand('node index.js', { + cwd: join(tmpProjPath(), 'test-cjs'), + }); + expect(output).toContain(esbuildLib); + expect(output).toContain(viteLib); + expect(output).toContain(rollupLib); + + // Make sure outputs in esm project + createFile( + 'test-esm/package.json', + JSON.stringify( + { + name: 'test-esm', + private: true, + type: 'module', + dependencies: { + [`@proj/${esbuildLib}`]: `file:../dist/libs/${esbuildLib}`, + [`@proj/${viteLib}`]: `file:../dist/libs/${viteLib}`, + [`@proj/${rollupLib}`]: `file:../dist/libs/${rollupLib}`, + }, + }, + null, + 2 + ) + ); + createFile( + 'test-esm/index.js', + ` + import { ${esbuildLib} } from '@proj/${esbuildLib}'; + import { ${viteLib} } from '@proj/${viteLib}'; + import { ${rollupLib} } from '@proj/${rollupLib}'; + console.log(${esbuildLib}()); + console.log(${viteLib}()); + console.log(${rollupLib}()); + ` + ); + runCommand(pmc.install, { + cwd: join(tmpProjPath(), 'test-esm'), + }); + output = runCommand('node index.js', { + cwd: join(tmpProjPath(), 'test-esm'), + }); + expect(output).toContain(esbuildLib); + expect(output).toContain(viteLib); + expect(output).toContain(rollupLib); + }, 500_000); + + it('should support tsc and swc for building libs', () => { + const tscLib = uniq('tsclib'); + const swcLib = uniq('swclib'); + + runCLI(`generate @nx/js:lib ${tscLib} --bundler=tsc --no-interactive`); + runCLI(`generate @nx/js:lib ${swcLib} --bundler=swc --no-interactive`); + + runCLI(`build ${tscLib}`); + runCLI(`build ${swcLib}`); + + const pmc = getPackageManagerCommand(); + let output: string; + + // Make sure outputs in commonjs project + createFile( + 'test-cjs/package.json', + JSON.stringify( + { + name: 'test-cjs', + private: true, + type: 'commonjs', + dependencies: { + [`@proj/${tscLib}`]: `file:../dist/libs/${tscLib}`, + [`@proj/${swcLib}`]: `file:../dist/libs/${swcLib}`, + }, + }, + null, + 2 + ) + ); + createFile( + 'test-cjs/index.js', + ` + const { ${tscLib} } = require('@proj/${tscLib}'); + const { ${swcLib} } = require('@proj/${swcLib}'); + console.log(${tscLib}()); + console.log(${swcLib}()); + ` + ); + runCommand(pmc.install, { + cwd: join(tmpProjPath(), 'test-cjs'), + }); + output = runCommand('node index.js', { + cwd: join(tmpProjPath(), 'test-cjs'), + }); + expect(output).toContain(tscLib); + expect(output).toContain(swcLib); + }, 500_000); +}); diff --git a/e2e/js/src/js-swc.test.ts b/e2e/js/src/js-swc.test.ts index f07b82f080ca8..128509947fb3c 100644 --- a/e2e/js/src/js-swc.test.ts +++ b/e2e/js/src/js-swc.test.ts @@ -91,6 +91,12 @@ describe('js e2e', () => { return json; }); + updateJson(`libs/${lib}/package.json`, (json) => { + // Delete automatically generated helper dependency to test legacy behavior. + delete json.dependencies['@swc/helpers']; + return json; + }); + runCLI( `build ${lib} --generateLockfile=true --updateBuildableProjectDepsInPackageJson` ); diff --git a/e2e/js/src/js-tsc.test.ts b/e2e/js/src/js-tsc.test.ts index ad2595c74f15e..487fa1344505c 100644 --- a/e2e/js/src/js-tsc.test.ts +++ b/e2e/js/src/js-tsc.test.ts @@ -129,6 +129,12 @@ describe('js e2e', () => { return json; }); + updateJson(`libs/${lib}/package.json`, (json) => { + // Delete automatically generated helper dependency to test legacy behavior. + delete json.dependencies.tslib; + return json; + }); + runCLI(`build ${lib} --updateBuildableProjectDepsInPackageJson`); expect(readJson(`dist/libs/${lib}/package.json`)).toHaveProperty( @@ -228,6 +234,12 @@ describe('package.json updates', () => { `; }); + updateJson(`libs/${lib}/package.json`, (json) => { + // Delete automatically generated helper dependency to test legacy behavior. + delete json.dependencies.tslib; + return json; + }); + runCLI(`build ${lib} --updateBuildableProjectDepsInPackageJson`); // Check that only 'react' exists, don't care about version diff --git a/e2e/linter/src/linter.test.ts b/e2e/linter/src/linter.test.ts index 8049632344cf4..47e8608119df1 100644 --- a/e2e/linter/src/linter.test.ts +++ b/e2e/linter/src/linter.test.ts @@ -452,12 +452,6 @@ describe('Linter', () => { ]; return json; }); - // Set this to false for now until the `@nx/js:lib` generator is updated to include ts/swc helpers by default. - // TODO(jack): Remove this once the above is addressed in another PR. - updateJson(`libs/${mylib}/tsconfig.lib.json`, (json) => { - json.compilerOptions.importHelpers = false; - return json; - }); updateJson(`libs/${mylib}/project.json`, (json) => { json.targets.lint.options.lintFilePatterns = [ `libs/${mylib}/**/*.ts`, @@ -471,7 +465,7 @@ describe('Linter', () => { it('should report dependency check issues', () => { const rootPackageJson = readJson('package.json'); const nxVersion = rootPackageJson.devDependencies.nx; - const tslibVersion = rootPackageJson.devDependencies['tslib']; + const tslibVersion = rootPackageJson.dependencies['tslib']; let out = runCLI(`lint ${mylib}`, { silenceError: true }); expect(out).toContain('All files pass linting'); @@ -486,9 +480,8 @@ describe('Linter', () => { // output should now report missing dependencies section out = runCLI(`lint ${mylib}`, { silenceError: true }); - expect(out).toContain( - 'Dependency sections are missing from the "package.json"' - ); + expect(out).toContain('they are missing'); + expect(out).toContain('@nx/devkit'); // should fix the missing section issue out = runCLI(`lint ${mylib} --fix`, { silenceError: true }); @@ -500,9 +493,12 @@ describe('Linter', () => { { "dependencies": { "@nx/devkit": "${nxVersion}", + "tslib": "${tslibVersion}", }, + "main": "./src/index.js", "name": "@proj/${mylib}", "type": "commonjs", + "typings": "./src/index.d.ts", "version": "0.0.1", } `); @@ -514,7 +510,7 @@ describe('Linter', () => { }); out = runCLI(`lint ${mylib}`, { silenceError: true }); expect(out).toContain( - `The version specifier does not contain the installed version of "@nx/devkit" package: ${nxVersion}` + 'version specifier does not contain the installed version of "@nx/devkit"' ); // should fix the version mismatch issue diff --git a/packages/js/src/generators/library/library.spec.ts b/packages/js/src/generators/library/library.spec.ts index 965f63e6752cc..21c6f00a1d6bc 100644 --- a/packages/js/src/generators/library/library.spec.ts +++ b/packages/js/src/generators/library/library.spec.ts @@ -45,6 +45,7 @@ describe('lib', () => { build: "echo 'implement build'", test: "echo 'implement test'", }, + dependencies: {}, }); expect(tree.exists('libs/my-lib/src/index.ts')).toBeTruthy(); diff --git a/packages/js/src/generators/library/library.ts b/packages/js/src/generators/library/library.ts index 72fca66b35a0e..df93737beb3ee 100644 --- a/packages/js/src/generators/library/library.ts +++ b/packages/js/src/generators/library/library.ts @@ -32,6 +32,8 @@ import { getImportPath } from '../../utils/get-import-path'; import { esbuildVersion, nxVersion, + swcHelpersVersion, + tsLibVersion, typesNodeVersion, } from '../../utils/versions'; import jsInitGenerator from '../init/init'; @@ -176,11 +178,13 @@ function addProject( if (options.bundler === 'esbuild') { projectConfiguration.targets.build.options.generatePackageJson = true; + projectConfiguration.targets.build.options.format = ['cjs']; } if (options.bundler === 'rollup') { projectConfiguration.targets.build.options.project = `${options.projectRoot}/package.json`; projectConfiguration.targets.build.options.compiler = 'swc'; + projectConfiguration.targets.build.options.format = ['cjs', 'esm']; } if (options.bundler === 'swc' && options.skipTypeCheck) { @@ -325,18 +329,25 @@ function createFiles(tree: Tree, options: NormalizedSchema, filesDir: string) { updateJson(tree, packageJsonPath, (json) => { json.name = options.importPath; json.version = '0.0.1'; - json.type = 'commonjs'; // If the package is publishable, we should remove the private field. if (json.private && options.publishable) { delete json.private; } - return json; + return { + ...json, + dependencies: { + ...json.dependencies, + ...determineDependencies(options), + }, + ...determineEntryFields(options), + }; }); } else { writeJson(tree, packageJsonPath, { name: options.importPath, version: '0.0.1', - type: 'commonjs', + dependencies: determineDependencies(options), + ...determineEntryFields(options), }); } @@ -619,5 +630,76 @@ function createProjectTsConfigJson(tree: Tree, options: NormalizedSchema) { ); } +function determineDependencies( + options: LibraryGeneratorSchema +): Record { + switch (options.bundler) { + case 'tsc': + // importHelpers is true by default, so need to add tslib as a dependency. + return { + tslib: tsLibVersion, + }; + case 'swc': + // externalHelpers is true by default, so need to add swc helpers as a dependency. + return { + '@swc/helpers': swcHelpersVersion, + }; + default: { + // In other cases (vite, rollup, esbuild), helpers are bundled so no need to add them as a dependency. + return {}; + } + } +} + +type EntryField = string | { [key: string]: EntryField }; + +function determineEntryFields( + options: LibraryGeneratorSchema +): Record { + switch (options.bundler) { + case 'tsc': + return { + type: 'commonjs', + main: './src/index.js', + typings: './src/index.d.ts', + }; + case 'swc': + return { + type: 'commonjs', + main: './src/index.js', + typings: './src/index.d.ts', + }; + case 'rollup': + return { + type: 'commonjs', + main: './index.cjs', + module: './index.js', + // typings is missing for rollup currently + }; + case 'vite': + return { + // Since we're publishing both formats, skip the type field. + // Bundlers or Node will determine the entry point to use. + main: './index.js', + module: './index.mjs', + typings: './index.d.ts', + }; + case 'esbuild': + // For libraries intended for Node, use CJS. + return { + type: 'commonjs', + main: './index.cjs', + // typings is missing for esbuild currently + }; + default: { + return { + // CJS is the safest optional for now due to lack of support from some packages + // also setting `type: module` results in different resolution behavior (e.g. import 'foo' no longer resolves to 'foo/index.js') + type: 'commonjs', + }; + } + } +} + export default libraryGenerator; export const librarySchematic = convertNxGenerator(libraryGenerator);