diff --git a/.changeset/nasty-guests-sniff.md b/.changeset/nasty-guests-sniff.md new file mode 100644 index 000000000..2155bf00c --- /dev/null +++ b/.changeset/nasty-guests-sniff.md @@ -0,0 +1,5 @@ +--- +"modular-scripts": patch +--- + +fix: package builds respect --private option diff --git a/.gitignore b/.gitignore index f334a8c10..29f0c1940 100644 --- a/.gitignore +++ b/.gitignore @@ -10,7 +10,10 @@ packages/sample-view packages/sample-package packages/nested packages/sample-library-package +packages/sample-renamable-library-package +packages/sample-renamed-library-package packages/sample-depending-package +packages/sample-renamable-depending-package # example app packages/esbuild-app diff --git a/packages/modular-scripts/src/__tests__/__fixtures__/packages/sample-depending-package/index.ts b/packages/modular-scripts/src/__tests__/__fixtures__/packages/sample-depending-package/index.ts new file mode 100644 index 000000000..b94185b24 --- /dev/null +++ b/packages/modular-scripts/src/__tests__/__fixtures__/packages/sample-depending-package/index.ts @@ -0,0 +1,8 @@ +/* eslint-disable @typescript-eslint/no-unsafe-call */ +/* eslint-disable @typescript-eslint/no-unsafe-return */ +// @ts-ignore +import add from 'sample-library-package'; + +export function double(x: number): number { + return add(x, x); +} diff --git a/packages/modular-scripts/src/__tests__/__fixtures__/packages/sample-depending-package/index.tsx b/packages/modular-scripts/src/__tests__/__fixtures__/packages/sample-depending-package/index.tsx deleted file mode 100644 index 594c13699..000000000 --- a/packages/modular-scripts/src/__tests__/__fixtures__/packages/sample-depending-package/index.tsx +++ /dev/null @@ -1,11 +0,0 @@ -import * as React from 'react'; -import * as ReactDOM from 'react-dom'; -// @ts-ignore -import App from './App'; - -ReactDOM.render( - - - , - document.getElementById('root'), -); diff --git a/packages/modular-scripts/src/__tests__/build.test.ts b/packages/modular-scripts/src/__tests__/build.test.ts index 68eaebc71..851e945ef 100644 --- a/packages/modular-scripts/src/__tests__/build.test.ts +++ b/packages/modular-scripts/src/__tests__/build.test.ts @@ -1,73 +1,29 @@ -import execa from 'execa'; -import { promisify } from 'util'; -import _rimraf from 'rimraf'; import tree from 'tree-view-for-tests'; import path from 'path'; import fs from 'fs-extra'; +import { addFixturePackage, cleanup, modular } from '../test/utils'; import getModularRoot from '../utils/getModularRoot'; -const rimraf = promisify(_rimraf); - const modularRoot = getModularRoot(); -function modular(str: string, opts: Record = {}) { - return execa('yarnpkg', ['modular', ...str.split(' ')], { - cwd: modularRoot, - cleanup: true, - ...opts, - }); -} - -async function cleanup() { - const packagesPath = path.join(getModularRoot(), 'packages'); - - await rimraf(path.join(packagesPath, 'sample-async-package')); - // run yarn so yarn.lock gets reset - await execa('yarnpkg', ['--silent'], { - cwd: modularRoot, - }); -} - -beforeAll(async () => { - await cleanup(); -}); - -afterAll(async () => { - await cleanup(); -}); - describe('WHEN building with preserve modules', () => { - beforeAll(async () => { - await modular( - 'add sample-async-package --unstable-type package --unstable-name sample-async-package', - { - stdio: 'inherit', - }, - ); - - const packageSrc = path.join( - getModularRoot(), - 'packages', - 'sample-async-package', - 'src', - ); - await fs.emptyDir(packageSrc); - await fs.copy( - path.join(__dirname, '__fixtures__', 'packages', 'sample-async-package'), - packageSrc, - ); + const packageName = 'sample-async-package'; - // build a package too, but preserve modules - await modular('build sample-async-package --preserve-modules', { + beforeAll(async () => { + await cleanup([packageName]); + await addFixturePackage(packageName); + await modular(`build ${packageName} --preserve-modules`, { stdio: 'inherit', }); }); + afterAll(async () => await cleanup([packageName])); + it('THEN expects the correct output package.json', async () => { expect( await fs.readJson( - path.join(modularRoot, 'dist', 'sample-async-package', 'package.json'), + path.join(modularRoot, 'dist', packageName, 'package.json'), ), ).toMatchInlineSnapshot(` Object { @@ -89,7 +45,7 @@ describe('WHEN building with preserve modules', () => { }); it('THEN expects the correct output directory structure', () => { - expect(tree(path.join(modularRoot, 'dist', 'sample-async-package'))) + expect(tree(path.join(modularRoot, 'dist', packageName))) .toMatchInlineSnapshot(` "sample-async-package ├─ README.md #1jv3l2q @@ -117,7 +73,7 @@ describe('WHEN building with preserve modules', () => { path.join( getModularRoot(), 'dist', - 'sample-async-package', + packageName, 'dist-es', 'index.js', ), @@ -133,7 +89,7 @@ describe('WHEN building with preserve modules', () => { path.join( getModularRoot(), 'dist', - 'sample-async-package', + packageName, 'dist-es', 'runAsync.js', ), @@ -142,3 +98,64 @@ describe('WHEN building with preserve modules', () => { ).toMatchSnapshot(); }); }); + +describe('WHEN building packages with private cross-package dependencies', () => { + const libraryPackage = 'sample-library-package'; + const dependentPackage = 'sample-depending-package'; + + beforeAll(async () => { + await cleanup([libraryPackage, dependentPackage]); + await addFixturePackage(libraryPackage); + await addFixturePackage(dependentPackage); + + // Can't specify it in the fixtures, as we can't nest package.json files in packages + const package_json = `{ + "name": "${libraryPackage}", + "version": "1.0.0", + "main": "src/index.ts", + "private": true, + "modular": { + "type": "package" + } +} +`; + + await fs.writeFile( + path.join(getModularRoot(), 'packages', libraryPackage, 'package.json'), + package_json, + 'utf8', + ); + }); + + afterAll(async () => await cleanup([dependentPackage, libraryPackage])); + + it('THEN the build fails by default', () => { + return expect( + async () => + await modular(`build ${dependentPackage} --preserve-modules`, { + stdio: 'inherit', + }), + ).rejects.toThrow(); + }); + + it('THEN the build passes if the --private option is used', async () => { + await modular(`build ${dependentPackage} --preserve-modules --private`, { + stdio: 'inherit', + }); + + expect(tree(path.join(modularRoot, 'dist', dependentPackage))) + .toMatchInlineSnapshot(` + "sample-depending-package + ├─ README.md #1jv3l2q + ├─ dist-cjs + │ ├─ index.js #1gj4b9h + │ └─ index.js.map #39c8bu + ├─ dist-es + │ ├─ index.js #xezjee + │ └─ index.js.map #89b1k5 + ├─ dist-types + │ └─ index.d.ts #6hjmh9 + └─ package.json" + `); + }); +}); diff --git a/packages/modular-scripts/src/__tests__/rename.test.ts b/packages/modular-scripts/src/__tests__/rename.test.ts index 549d9dbea..28af63418 100644 --- a/packages/modular-scripts/src/__tests__/rename.test.ts +++ b/packages/modular-scripts/src/__tests__/rename.test.ts @@ -1,157 +1,64 @@ -import execa from 'execa'; -import { promisify } from 'util'; -import _rimraf from 'rimraf'; -import path from 'path'; import fs from 'fs-extra'; +import path from 'path'; +import { addFixturePackage, cleanup, modular } from '../test/utils'; import getModularRoot from '../utils/getModularRoot'; -const rimraf = promisify(_rimraf); - const modularRoot = getModularRoot(); -function modular(str: string, opts: Record = {}) { - return execa('yarnpkg', ['modular', ...str.split(' ')], { - cwd: modularRoot, - cleanup: true, - ...opts, - }); -} - -async function cleanup() { - const packagesPath = path.join(getModularRoot(), 'packages'); - - await rimraf(path.join(packagesPath, 'sample-depending-package')); - await rimraf(path.join(packagesPath, 'sample-library-package')); - // run yarn so yarn.lock gets reset - await execa('yarnpkg', ['--silent'], { - cwd: modularRoot, - }); -} - -beforeAll(async () => { - await cleanup(); -}); - -afterAll(async () => { - await cleanup(); -}); - describe('Rename command', () => { - beforeAll(async () => { - await modular( - 'add sample-depending-package --unstable-type package --unstable-name sample-depending-package', - { - stdio: 'inherit', - }, - ); - - await modular( - 'add sample-library-package --unstable-type package --unstable-name sample-library-package', - { - stdio: 'inherit', - }, - ); - - const libraryPackageSrc = path.join( - getModularRoot(), - 'packages', - 'sample-library-package', - 'src', - ); - await fs.emptyDir(libraryPackageSrc); - await fs.copy( - path.join( - __dirname, - '__fixtures__', - 'packages', - 'sample-library-package', - ), - libraryPackageSrc, - ); - - const dependingPackageSrc = path.join( - getModularRoot(), - 'packages', - 'sample-depending-package', - 'src', - ); - await fs.emptyDir(dependingPackageSrc); - await fs.copy( - path.join( - __dirname, - '__fixtures__', - 'packages', - 'sample-depending-package', - ), - dependingPackageSrc, - ); - - // Can't specify it in the fixtures, otherwise rename will rename the 'sample-library-package' dependency! - const app_tsx = ` - import * as React from 'react'; - import summer from 'sample-library-package'; - - declare function summer(a: number, b: number): number; - function App(): JSX.Element { - const result = summer(7, 7); - return ( -
-
-

This is the result: {result}

-
-
- ); - } + const libraryPackage = 'sample-renamable-library-package'; + const renamedPackage = 'sample-renamed-library-package'; + const dependentPackage = 'sample-renamable-depending-package'; - export default App; -`; + beforeAll(async () => { + await cleanup([dependentPackage, libraryPackage, renamedPackage]); + await addFixturePackage(libraryPackage, { copy: false }); + await addFixturePackage(dependentPackage, { copy: false }); + // For now, we have to write the index.ts file here because if we store it + // as a fixture, the fixture itself will be renamed during our test await fs.writeFile( - path.join(dependingPackageSrc, 'App.tsx'), - app_tsx, - 'utf8', + path.join(modularRoot, 'packages', dependentPackage, 'src', 'index.ts'), + `/* eslint-disable @typescript-eslint/no-unsafe-call */ +/* eslint-disable @typescript-eslint/no-unsafe-return */ +// @ts-ignore +import add from '${libraryPackage}'; + +export function double(x: number): number { + return add(x, x); +} +`, ); }); + afterAll( + async () => + await cleanup([dependentPackage, libraryPackage, renamedPackage]), + ); + it('expects file importing the dependency to refer to the renamed dep', async () => { - await modular( - 'rename sample-library-package sample-library-renamed-package', - { - stdio: 'inherit', - }, - ); + await modular(`rename ${libraryPackage} ${renamedPackage}`, { + stdio: 'inherit', + }); + + const snapshot = ` + "/* eslint-disable @typescript-eslint/no-unsafe-call */ + /* eslint-disable @typescript-eslint/no-unsafe-return */ + // @ts-ignore + import add from '${renamedPackage}'; + + export function double(x: number): number { + return add(x, x); + } + " + `; expect( await fs.readFile( - path.join( - modularRoot, - 'packages', - 'sample-depending-package', - 'src', - 'App.tsx', - ), + path.join(modularRoot, 'packages', dependentPackage, 'src', 'index.ts'), 'utf8', ), - ).toMatchInlineSnapshot(` - " - import * as React from 'react'; - import summer from 'sample-library-renamed-package'; - - declare function summer(a: number, b: number): number; - function App(): JSX.Element { - const result = summer(7, 7); - return ( -
-
-

This is the result: {result}

-
-
- ); - } - - export default App; - " - `); + ).toMatchInlineSnapshot(snapshot); }); }); diff --git a/packages/modular-scripts/src/build/buildPackage/makeBundle.ts b/packages/modular-scripts/src/build/buildPackage/makeBundle.ts index caf00ab09..485023edf 100644 --- a/packages/modular-scripts/src/build/buildPackage/makeBundle.ts +++ b/packages/modular-scripts/src/build/buildPackage/makeBundle.ts @@ -183,7 +183,10 @@ export async function makeBundle( // Let's collect the name and add it in the package.json // we publish to the registry // TODO: make sure local workspaces are NOT explicitly included in package.json - if (packageJsons[importedPackage].private !== true) { + if ( + includePrivate || + packageJsons[importedPackage].private !== true + ) { localImports[importedPackage] = packageJsons[importedPackage] .version as string; } else { diff --git a/packages/modular-scripts/src/test/utils.ts b/packages/modular-scripts/src/test/utils.ts new file mode 100644 index 000000000..5087d5be5 --- /dev/null +++ b/packages/modular-scripts/src/test/utils.ts @@ -0,0 +1,55 @@ +import _rimraf from 'rimraf'; +import execa from 'execa'; +import fs from 'fs-extra'; +import path from 'path'; +import { promisify } from 'util'; + +import getModularRoot from '../utils/getModularRoot'; + +const rimraf = promisify(_rimraf); + +const modularRoot = getModularRoot(); + +export function modular( + str: string, + opts: Record = {}, +): execa.ExecaChildProcess { + return execa('yarnpkg', ['modular', ...str.split(' ')], { + cwd: modularRoot, + cleanup: true, + ...opts, + }); +} + +export async function cleanup(packageNames: Array): Promise { + const packagesPath = path.join(modularRoot, 'packages'); + const distPath = path.join(modularRoot, 'dist'); + + for (const packageName of packageNames) { + await rimraf(path.join(packagesPath, packageName)); + await rimraf(path.join(distPath, packageName)); + } + + // run yarn so yarn.lock gets reset + await execa('yarnpkg', ['--silent'], { + cwd: modularRoot, + }); +} + +export async function addFixturePackage( + name: string, + options: { copy: boolean } = { copy: true }, +): Promise { + const packageSrcDir = path.join(modularRoot, 'packages', name, 'src'); + await modular(`add ${name} --unstable-type package --unstable-name ${name}`, { + stdio: 'inherit', + }); + await fs.emptyDir(packageSrcDir); + + if (options.copy) { + await fs.copy( + path.join(__dirname, '..', '__tests__', '__fixtures__', 'packages', name), + packageSrcDir, + ); + } +}