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

feat(nuxi): cli wrapper for self restart #18641

Merged
merged 9 commits into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion docs/3.api/4.advanced/1.hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Hook | Arguments | Description
`kit:compatibility` | `compatibility, issues` | Allows extending compatibility checks.
`ready` | `nuxt` | Called after Nuxt initialization, when the Nuxt instance is ready to work.
`close` | `nuxt` | Called when Nuxt instance is gracefully closing.
`restart` | - | Called to restart the current Nuxt instance. **This hook is currently only available on the [Edge Channel](/docs/guide/going-further/edge-channel/).** <!-- stabilityedge -->
`restart` | `{ hard?: boolean }` | To be called to restart the current Nuxt instance. **This hook is currently only available on the [Edge Channel](/docs/guide/going-further/edge-channel/).** <!-- stabilityedge -->
`modules:before` | - | Called during Nuxt initialization, before installing user modules.
`modules:done` | - | Called during Nuxt initialization, after installing user modules.
`app:resolve` | `app` | Called after resolving the `app` instance.
Expand Down
3 changes: 1 addition & 2 deletions packages/nuxi/bin/nuxi.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
#!/usr/bin/env node
process._startTime = Date.now()
import('../dist/cli.mjs').then(r => (r.default || r).main())
import('../dist/cli-wrapper.mjs')
2 changes: 2 additions & 0 deletions packages/nuxi/build.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ export default defineBuildConfig({
},
entries: [
'src/cli',
'src/cli-run',
'src/cli-wrapper',
'src/index'
],
externals: [
Expand Down
4 changes: 4 additions & 0 deletions packages/nuxi/src/cli-run.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// @ts-ignore
process._startTime = Date.now()
// @ts-ignore
import('./cli').then(r => (r.default || r).main())
40 changes: 40 additions & 0 deletions packages/nuxi/src/cli-wrapper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* This file is used to wrap the CLI entrypoint in a restartable process.
*/
import { fileURLToPath } from 'node:url'
import { execa } from 'execa'
import { EXIT_CODE_RESTART } from './constants'

const cliEntry = fileURLToPath(new URL('../dist/cli-run.mjs', import.meta.url))

async function startSubprocess (preArgs: string[], postArgs: string[]) {
const child = await execa(
Copy link
Member

@pi0 pi0 Mar 2, 2023

Choose a reason for hiding this comment

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

Perf: I guess we can use fork (or exec) from nod:child_process. This is also consistent with the final nuxi-ng implementation and provides a communication channel.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see we use execa in many places already, and the chunk is shared. I guess the performance won't be a big issue here.

I am just worried about using fork or exec might create more issues to handle on different OS, etc.

Copy link
Member

@pi0 pi0 Mar 3, 2023

Choose a reason for hiding this comment

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

Main usecase i added execa was search path compatibility which isn’t a case here. Also fork copies mem in unix then execs which by theory at least should be faster than loading node to the mem again.

Copy link
Member

Choose a reason for hiding this comment

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

Could we mesure the performance difference here?

I like the simplicity of code and execa has some good features:
CleanShot 2023-03-03 at 10 12 41@2x

Copy link
Member

@pi0 pi0 Mar 3, 2023

Choose a reason for hiding this comment

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

Most of those benefits aren't relevant here and main benefit is using fork over exec with fine gained control. we are forking a known entry as process (not a on-off executation like other commands).

Finally this impl will be part of citty's dev server so this change is to make sure we are aligned :)

'node',
[
...preArgs,
cliEntry,
...postArgs
],
{
reject: false,
stdio: 'inherit',
env: {
...process.env,
NUXI_CLI_WRAPPER: 'true'
}
}
)
if (child.exitCode === EXIT_CODE_RESTART) {
await startSubprocess(preArgs, postArgs)
} else {
process.exit(child.exitCode)
}
}

const args = process.argv.slice(2)
// only enable wrapper in dev command
if (args[0] === 'dev') {
await startSubprocess([], args)
} else {
await import(cliEntry)
}
9 changes: 9 additions & 0 deletions packages/nuxi/src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { loadKit } from '../utils/kit'
import { importModule } from '../utils/cjs'
import { overrideEnv } from '../utils/env'
import { writeNuxtManifest, loadNuxtManifest, cleanupNuxtDirs } from '../utils/nuxt'
import { EXIT_CODE_RESTART } from '../constants'
import { defineNuxtCommand } from './index'

export default defineNuxtCommand({
Expand Down Expand Up @@ -89,6 +90,14 @@ export default defineNuxtCommand({
}

currentNuxt = await loadNuxt({ rootDir, dev: true, ready: false })
// Hard restart
if (process.env.NUXI_CLI_WRAPPER) {
currentNuxt.hooks.hook('restart', (options) => {
if (options?.hard) {
process.exit(EXIT_CODE_RESTART)
}
})
}
currentNuxt.hooks.hookOnce('restart', () => load(true))

if (!isRestart) {
Expand Down
11 changes: 11 additions & 0 deletions packages/nuxi/src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Special exit code to restart the process
*
* Usage:
* ```ts
* if (process.env.NUXI_CLI_WRAPPER) {
* process.exit(EXIT_CODE_RESTART)
* }
* ```
*/
export const EXIT_CODE_RESTART = 85
1 change: 1 addition & 0 deletions packages/nuxi/src/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './run'
export * from './constants'
7 changes: 6 additions & 1 deletion packages/schema/src/types/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ export interface NuxtHooks {
* Called to restart the current Nuxt instance.
* @returns Promise
*/
'restart': () => HookResult
'restart': (options?: {
/**
* Try to restart the whole process if supported
*/
hard?: boolean
}) => HookResult

/**
* Called during Nuxt initialization, before installing user modules.
Expand Down