-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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(gatsby-dev-cli): Correctly catch 404s from NPM when a package hasn't been published yet #28297
Conversation
…n't been published yet
got(`https://unpkg.com/${packageName}/package.json`) | ||
.then((error, response) => { | ||
if (response && response.statusCode === 200) { | ||
return resolve(JSON.parse(response.body)) | ||
resolve(JSON.parse(response.body)) | ||
} | ||
|
||
return reject(error) | ||
} | ||
) | ||
}) | ||
.catch(e => reject(e)) |
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.
.then
handler can only have single argument - so this should be:
got(`https://unpkg.com/${packageName}/package.json`)
.then(response => {
if (response && response.statusCode === 200) {
return resolve(JSON.parse(response.body))
}
return reject(new Error(`No response or non 200 code`))
})
.catch(e => reject(e))
Seems like migration in #26653 was never actually tested?
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.
Considering this is inside an async
function the whole new Promise
expression should be replaced with something like
const response = await got(`https://unpkg.com/${packageName}/package.json`)
if (response?.statusCode !== 200) {
throw new Error(`No response or non 200 code`)
}
localPKGjson = JSON.parse(response.body)
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.
(unless got
does not return a proper promise-like?)
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.
Ah, yeah, you are probably right and await
will simplify things - I mostly zoomed in on double args for .then
:)
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.
Seems like migration in #26653 was never actually tested?
Yeah — this isn't a common scenario.
Will change to await 👍
Co-authored-by: Peter van der Zee <209817+pvdz@users.noreply.github.com>
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.
Thanks
Can't use gatsby-dev-cli with new package otherwise