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.

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

Jump to bottom

fix(next/dev): do not suppress error from bindings #41989

Merged
merged 3 commits into from Oct 27, 2022
Merged

fix(next/dev): do not suppress error from bindings #41989

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b11595
fix(next/dev): do not suppress error from bindings
kwonoj Oct 27, 2022
0d94e66
Merge branch 'canary' into fix-suppress-err
kodiakhq[bot] Oct 27, 2022
2da5751
Merge branch 'canary' into fix-suppress-err
kodiakhq[bot] Oct 27, 2022
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
13 changes: 10 additions & 3 deletions packages/next/cli/next-dev.ts
View file
Open in desktop
Expand Up @@ -103,7 +103,7 @@ const nextDev: cliCommand = (argv) => {

if (args['--turbo']) {
// check for postcss, babelrc, swc plugins
return new Promise(async (resolve) => {
return new Promise<void>(async (resolve) => {
const { findConfigPath } =
require('../lib/find-config') as typeof import('../lib/find-config')
const { loadBindings } =
Expand Down Expand Up @@ -260,7 +260,7 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit
}
console.log(feedbackMessage)

loadBindings()
return loadBindings()
.then((bindings: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we wrapping in a async function with a promise constructor? Can we just use the await syntax and wrap in a try/catch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's mostly leftover from last-minute error message changes and we may refactor this logic bit later, including fixing some handlings in error message part itself.

// eslint-disable-next-line no-shadow
const findUp =
Expand All @@ -286,7 +286,14 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit
preflight().catch(() => {})
return server
})
.then(resolve)
.then(
() => {
resolve()
},
(err: Error) => {
console.error(err)
}
)
})
} else {
startServer(devServerOptions)
Expand Down