-
Notifications
You must be signed in to change notification settings - Fork 27.1k
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
Add option to pass --inspect flag to enable server-side debugging #3294
Conversation
I think it should also handle --inspect-brk, and --inspect=1234 |
bin/next
Outdated
|
||
const startProcess = () => { | ||
const proc = spawn(bin, args, { stdio: 'inherit', customFds: [0, 1, 2] }) | ||
const proc = spawn(node, args, { stdio: 'inherit', customFds: [0, 1, 2] }) |
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.
Shouldn't this be bin
? = const bin = join(__dirname, 'next-' + cmd)
🤔
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.
No. I've changed the scripts to be regular JavaScript files, that doesn't contain #!/usr/bin/env node
as we need to launch Node with the --inspect
flag and having the files as a shell script doesn't allow that.
So the change here is the that we spawn a child process like node --inspect <path to next-dev>
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.
This is awesome, thank you very much @auchenberg ❤️ Just had one question.
@auchenberg can you check the CI results 👍 |
bin/next-build
Outdated
@@ -1,4 +1,3 @@ | |||
#!/usr/bin/env node |
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.
Do you really need to change this? While I have never seen anyone running next-build
or next-dev
commands (instead of next build
/ next dev
, where next
command gets executed), this seems to be a breaking change to me, and given the files locate in the bin
folder, it makes sense to be able to execute them (so shebang is necessary).
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.
I don't like this change their, but I haven't found a way to pass the --inspect
flag to #!/usr/bin/env node
otherwise.
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.
@auchenberg having this shebang doesn't require you use them. Just leave shebang even if you are going to run them from next
using explicit node --inspect next-build
.
@timneutkens @frol The PR has been simplified, and CI is passing now. Windows CI seems to be a bit flaky. |
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.
Awesome!
Hello from the VS @code team,
I'm currently looking into enabling next.js to debugged from VS Code in microsoft/vscode-recipes#31, and after some research I didn't seem possible to start
next
with Node's--inspect
flag that starts the Node.js debugging server. This PR modifies the CLI to enable the ``--inspect` flag to be passed to the spawned child process, and enables debugging of the server-side rendering in VS Code (and Chrome DevTools).Steps to validate in Chrome DevTools:
next --inspect
ornext dev --inspect
getInitialProps
, which is used for SSR.Add option to pass --inspect flag to enable server-side debugging