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(reporter): update notifier instructions #5750

Merged
merged 2 commits into from Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/flat-rabbits-smile.md
@@ -0,0 +1,6 @@
---
"@pnpm/default-reporter": patch
"pnpm": patch
---

The update notifier should suggest using the standalone script, when pnpm was installed using a standalone script.
3 changes: 3 additions & 0 deletions cli/default-reporter/src/index.ts
Expand Up @@ -28,6 +28,7 @@ export function initDefaultReporter (
argv: string[]
config?: Config
env?: NodeJS.ProcessEnv
process?: NodeJS.Process
}
}
): () => void {
Expand Down Expand Up @@ -95,6 +96,7 @@ export function toOutput$ (
argv: string[]
config?: Config
env?: NodeJS.ProcessEnv
process?: NodeJS.Process
}
}
): Rx.Observable<string> {
Expand Down Expand Up @@ -235,6 +237,7 @@ export function toOutput$ (
cmd: opts.context.argv[0],
config: opts.context.config,
env: opts.context.env ?? process.env,
process: opts.context.process ?? process,
isRecursive: opts.context.config?.['recursive'] === true,
logLevel: opts.reportingOptions?.logLevel,
pnpmConfig: opts.context.config,
Expand Down
1 change: 1 addition & 0 deletions cli/default-reporter/src/reporterForClient/index.ts
Expand Up @@ -57,6 +57,7 @@ export function reporterForClient (
cmd: string
config?: Config
env: NodeJS.ProcessEnv
process: NodeJS.Process
isRecursive: boolean
logLevel?: LogLevel
pnpmConfig?: Config
Expand Down
28 changes: 24 additions & 4 deletions cli/default-reporter/src/reporterForClient/reportUpdateCheck.ts
Expand Up @@ -7,20 +7,22 @@ import semver from 'semver'

export function reportUpdateCheck (log$: Rx.Observable<UpdateCheckLog>, opts: {
env: NodeJS.ProcessEnv
process: NodeJS.Process
}) {
return log$.pipe(
take(1),
filter((log) => semver.gt(log.latestVersion, log.currentVersion)),
map((log) => {
const updateCommand = renderUpdateCommand({
const updateMessage = renderUpdateMessage({
currentPkgIsExecutable: detectIfCurrentPkgIsExecutable(opts.process),
latestVersion: log.latestVersion,
env: opts.env,
})
return Rx.of({
msg: boxen(`\
Update available! ${chalk.red(log.currentVersion)} → ${chalk.green(log.latestVersion)}.
${chalk.magenta('Changelog:')} https://github.com/pnpm/pnpm/releases/tag/v${log.latestVersion}
Run "${chalk.magenta(updateCommand)}" to update.
${updateMessage}

Follow ${chalk.magenta('@pnpmjs')} for updates: https://twitter.com/pnpmjs`,
{
Expand All @@ -36,10 +38,28 @@ Follow ${chalk.magenta('@pnpmjs')} for updates: https://twitter.com/pnpmjs`,
)
}

function renderUpdateCommand (opts: { env: NodeJS.ProcessEnv, latestVersion: string }) {
interface UpdateMessageOptions {
currentPkgIsExecutable: boolean
env: NodeJS.ProcessEnv
latestVersion: string
}

function renderUpdateMessage (opts: UpdateMessageOptions) {
if (opts.currentPkgIsExecutable && opts.env.PNPM_HOME) {
return 'Run one of the standalone scripts from: https://pnpm.io/installation'
}
const updateCommand = renderUpdateCommand(opts)
return `Run "${chalk.magenta(updateCommand)}" to update.`
}

function renderUpdateCommand (opts: UpdateMessageOptions) {
if (opts.env.COREPACK_ROOT) {
return `corepack prepare pnpm@${opts.latestVersion} --activate`
}
const pkgName = process['pkg'] != null ? '@pnpm/exe' : 'pnpm'
const pkgName = opts.currentPkgIsExecutable ? '@pnpm/exe' : 'pnpm'
return `pnpm add -g ${pkgName}`
}

function detectIfCurrentPkgIsExecutable (process: NodeJS.Process) {
return process['pkg'] != null
}
Expand Up @@ -27,3 +27,17 @@ exports[`print update notification if the latest version is greater than the cur
╰──────────────────────────────────────────────────────────────────╯
"
`;

exports[`print update notification that suggests to use the standalone scripts for the upgrade 1`] = `
"
╭──────────────────────────────────────────────────────────────────────────╮
│ │
│ Update available! 10.0.0 → 11.0.0. │
│ Changelog: https://github.com/pnpm/pnpm/releases/tag/v11.0.0 │
│ Run one of the standalone scripts from: https://pnpm.io/installation │
│ │
│ Follow @pnpmjs for updates: https://twitter.com/pnpmjs │
│ │
╰──────────────────────────────────────────────────────────────────────────╯
"
`;
Copy link
Member

Choose a reason for hiding this comment

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

Need four spaces to match snapshots

Copy link
Member Author

Choose a reason for hiding this comment

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

Locally the test is passing. So I am not sure what is the problem.

31 changes: 31 additions & 0 deletions cli/default-reporter/test/reportingUpdateCheck.ts
Expand Up @@ -86,3 +86,34 @@ test('print update notification for Corepack if the latest version is greater th
},
})
})

test('print update notification that suggests to use the standalone scripts for the upgrade', (done) => {
const output$ = toOutput$({
context: {
argv: ['install'],
config: { recursive: true } as Config,
env: {
PNPM_HOME: '/home/user/.local/share/pnpm',
},
process: {
pkg: true,
} as any, // eslint-disable-line
},
streamParser: createStreamParser(),
})

updateCheckLogger.debug({
currentVersion: '10.0.0',
latestVersion: '11.0.0',
})

expect.assertions(1)

output$.pipe(take(1)).subscribe({
complete: () => done(),
error: done,
next: output => {
expect(stripAnsi(output)).toMatchSnapshot()
},
})
})