Skip to content

feat(auth): open the browser immediately instead of waiting for ENTER during login #9711

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.

Sign up for GitHub

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

Open
styfle wants to merge 1 commit into
base: latest
Choose a base branch
from
+7 −89
Open

feat(auth): open the browser immediately instead of waiting for ENTER during login #9711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 6 additions & 28 deletions lib/utils/open-url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
const { open } = require('@npmcli/promise-spawn')
const { output, input, META } = require('proc-log')
const { URL } = require('node:url')
const readline = require('node:readline/promises')
const { once } = require('node:events')

const assertValidUrl = (url) => {
try {
Expand Down Expand Up @@ -51,8 +49,8 @@ const openUrl = async (npm, url, title, isFile) => {
}
}

// Prompt to open URL in browser if possible
const openUrlPrompt = async (npm, url, title, prompt, { signal }) => {
// Print the url and open it in the browser if the environment is interactive

@cb1kenobi cb1kenobi Jul 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
// Print the url and open it in the browser if the environment is interactive
// Print the URL and open it in the browser if the environment is interactive

const openUrlPrompt = async (npm, url, title) => {
const browser = npm.config.get('browser')
const json = npm.config.get('json')

Expand All @@ -63,32 +61,12 @@ const openUrlPrompt = async (npm, url, title, prompt, { signal }) => {
return
}

const rl = readline.createInterface({
input: process.stdin,
output: process.stdout,
})

try {
await input.read(() => Promise.race([
rl.question(prompt, { signal }),
once(rl, 'error'),
once(rl, 'SIGINT').then(() => {
throw new Error('canceled')
}),
]))
rl.close()
await openUrl(npm, url, 'Browser unavailable. Please open the URL manually')
} catch (err) {
rl.close()
if (err.name !== 'AbortError') {
throw err
}
}
await openUrl(npm, url, 'Browser unavailable. Please open the URL manually')
}

// Rearrange arguments and return a function that takes the two arguments returned from the npm-profile methods that take an opener
const createOpener = (npm, title, prompt = 'Press ENTER to open in the browser...') =>
(url, opts) => openUrlPrompt(npm, url, title, prompt, opts)
// Rearrange arguments and return a function matching the opener signature expected by the npm-profile methods
const createOpener = (npm, title) =>
(url) => openUrlPrompt(npm, url, title)

module.exports = {
openUrl,
Expand Down
62 changes: 1 addition & 61 deletions test/lib/utils/open-url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const t = require('tap')
const tmock = require('../../fixtures/tmock')
const mockNpm = require('../../fixtures/mock-npm')
const EventEmitter = require('node:events')

const mockOpenUrl = async (t, args, { openerResult, ...config } = {}) => {
let openerUrl = null
Expand Down Expand Up @@ -38,11 +37,8 @@ const mockOpenUrl = async (t, args, { openerResult, ...config } = {}) => {
}

const mockOpenUrlPrompt = async (t, {
questionShouldResolve = true,
openUrlPromptInterrupted = false,
openerResult = null,
isTTY = true,
abort = false,
url: openUrl = 'https://www.npmjs.com',
...config
}) => {
Expand All @@ -67,41 +63,10 @@ const mockOpenUrlPrompt = async (t, {
}
},
},
'node:readline/promises': {
createInterface: () => {
return Object.assign(new EventEmitter(), {
question: async (p, { signal } = {}) => {
if (questionShouldResolve !== true) {
await new Promise((res, rej) => {
if (signal) {
signal.addEventListener('abort', () => {
const err = new Error('abort')
err.name = 'AbortError'
rej(err)
})
}
})
}
},
close: () => {},
once: function (event, cb) {
if (openUrlPromptInterrupted && event === 'SIGINT') {
cb()
}
},
})
},
},
})

let error
const abortController = new AbortController()
const args = [mock.npm, openUrl, 'npm home', 'prompt', { signal: abortController.signal }]
if (abort) {
mock.open = openUrlPrompt(...args)
} else {
await openUrlPrompt(...args).catch((er) => error = er)
}
await openUrlPrompt(mock.npm, openUrl, 'npm home').catch((er) => error = er)

mock.npm.finish()

Expand All @@ -111,7 +76,6 @@ const mockOpenUrlPrompt = async (t, {
openerOpts,
OUTPUT: mock.joinedOutput(),
error,
abortController,
}
}

Expand Down Expand Up @@ -157,20 +121,6 @@ t.test('open url prompt', async t => {
t.same(OUTPUT, '', 'printed no output')
})

t.test('does not open url if canceled', async t => {
const { openerUrl, openerOpts, open, abortController } = await mockOpenUrlPrompt(t, {
questionShouldResolve: false,
abort: true,
})

abortController.abort()

await open

t.equal(openerUrl, null, 'did not open')
t.same(openerOpts, null, 'did not open')
})

t.test('returns error when opener errors', async t => {
const { error, openerUrl } = await mockOpenUrlPrompt(t, {
openerResult: Object.assign(new Error('Opener failed'), { code: 1 }),
Expand All @@ -189,16 +139,6 @@ t.test('open url prompt', async t => {
t.equal(openerUrl, 'https://www.npmjs.com', 'did not open')
t.matchSnapshot(OUTPUT, 'Outputs extra Browser unavailable message and url')
})

t.test('throws "canceled" error on SIGINT', async t => {
const { open } = await mockOpenUrlPrompt(t, {
questionShouldResolve: false,
openUrlPromptInterrupted: true,
abort: true,
})

await t.rejects(open, /canceled/, 'message is canceled')
})
})

t.test('open url', async t => {
Expand Down
Loading