-
-
Notifications
You must be signed in to change notification settings - Fork 937
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
feat(deploy): apply publishConfig to all packages during deploy #6943
Changes from all commits
b12b3e9
c55f849
474d2c4
24b699f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
"@pnpm/plugin-commands-deploy": minor | ||
"@pnpm/directory-fetcher": minor | ||
--- | ||
|
||
Apply `publishConfig` for workspace packages on directory fetch. Enables a publishable ("exportable") `package.json` on deployment [#6693](https://github.com/pnpm/pnpm/issues/6693). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"name": "exportable", | ||
"version": "1.0.0", | ||
"main": "src/index.ts", | ||
"publishConfig": { | ||
"main": "dist/index.js" | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,15 @@ import path from 'path' | |
import { deploy } from '@pnpm/plugin-commands-deploy' | ||
import { assertProject } from '@pnpm/assert-project' | ||
import { preparePackages } from '@pnpm/prepare' | ||
import { REGISTRY_MOCK_PORT } from '@pnpm/registry-mock' | ||
import { readProjects } from '@pnpm/filter-workspace-packages' | ||
import crossSpawn from 'cross-spawn' | ||
import { sync as loadJsonFile } from 'load-json-file' | ||
import writeYamlFile from 'write-yaml-file' | ||
import { DEFAULT_OPTS } from './utils' | ||
|
||
const pnpmBin = path.join(__dirname, '../../../pnpm/bin/pnpm.cjs') | ||
|
||
test('deploy', async () => { | ||
preparePackages([ | ||
{ | ||
|
@@ -20,6 +26,10 @@ test('deploy', async () => { | |
'project-3': 'workspace:*', | ||
'is-negative': '1.0.0', | ||
}, | ||
main: 'local-file-1.js', | ||
publishConfig: { | ||
main: 'publish-file-1.js', | ||
}, | ||
}, | ||
{ | ||
name: 'project-2', | ||
|
@@ -29,6 +39,10 @@ test('deploy', async () => { | |
'project-3': 'workspace:*', | ||
'is-odd': '1.0.0', | ||
}, | ||
main: 'local-file-2.js', | ||
publishConfig: { | ||
main: 'publish-file-2.js', | ||
}, | ||
}, | ||
{ | ||
name: 'project-3', | ||
|
@@ -46,6 +60,10 @@ test('deploy', async () => { | |
fs.writeFileSync(`${name}/index.js`, '', 'utf8') | ||
}) | ||
|
||
await writeYamlFile('pnpm-workspace.yaml', { packages: ['*'] }) | ||
crossSpawn.sync(pnpmBin, ['install', '--ignore-scripts', '--store-dir=../store', `--registry=http://localhost:${REGISTRY_MOCK_PORT}`]) | ||
fs.rmSync('pnpm-lock.yaml') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A little bit of a hack for testing purposes... an assertion below checks that this file does not exist (as a side effect of the |
||
|
||
const { allProjects, selectedProjectsGraph } = await readProjects(process.cwd(), [{ namePattern: 'project-1' }]) | ||
|
||
await deploy.handler({ | ||
|
@@ -73,6 +91,26 @@ test('deploy', async () => { | |
expect(fs.existsSync('deploy/node_modules/.pnpm/file+project-3/node_modules/project-3/index.js')).toBeTruthy() | ||
expect(fs.existsSync('deploy/node_modules/.pnpm/file+project-3/node_modules/project-3/test.js')).toBeFalsy() | ||
expect(fs.existsSync('pnpm-lock.yaml')).toBeFalsy() // no changes to the lockfile are written | ||
const project1Manifest = loadJsonFile('deploy/package.json') | ||
expect(project1Manifest).toMatchObject({ | ||
name: 'project-1', | ||
main: 'publish-file-1.js', | ||
dependencies: { | ||
'is-positive': '1.0.0', | ||
'project-2': '2.0.0', | ||
}, | ||
}) | ||
expect(project1Manifest).not.toHaveProperty('publishConfig') | ||
const project2Manifest = loadJsonFile('deploy/node_modules/.pnpm/file+project-2/node_modules/project-2/package.json') | ||
expect(project2Manifest).toMatchObject({ | ||
name: 'project-2', | ||
main: 'publish-file-2.js', | ||
dependencies: { | ||
'project-3': '2.0.0', | ||
'is-odd': '1.0.0', | ||
}, | ||
}) | ||
expect(project2Manifest).not.toHaveProperty('publishConfig') | ||
}) | ||
|
||
test('deploy with dedupePeerDependents=true ignores the value of dedupePeerDependents', async () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to
createExportableManifest
we need metadata only available after an install.Presently,
deploy
does not actually require an install previously.This could be considered a breaking change. Although I would assume most users already have the relevant packages installed, and expected this behavior. This is inline with how commands like
publish
work.deploy
could be considered a "local"publish
.