-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
inspector: leverage Chrome DevTools' builtin support for NodeJS #10115
Conversation
Also change frontend URL from inspector.html to js_app.html
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.
LGTM
Currently working on the NiM project which is a UI for debugging V8 (Node.js and Deno) and this PR actually answers two questions have had regarding changes to this project! The first being why "deno" was changed to "node" in the info json endpoint, which happened to break some things on our end, and also more recently why I lost devtools debugging features. To both issues, I simply created workarounds in NiM. But it seems like a better solution might be available other than intentionally misnaming the runtime, like maybe adding something instead or in addition to? Idk. I have to say that it was one of those cases where I thought I remembered it being a certain way and then it wasn't, but I started doubting my memory... The second issue has to do with Deno only providing a single devtoolsFrontendUrl endpoint vs Node which provides a compatibility endpoint as well... see: https://github.com/nodejs/node/blob/main/src/inspector_socket_server.cc#L376-L384 deno/runtime/inspector_server.rs Lines 412 to 417 in 896dd56
Might it be useful to also have both endpoints here? It certainly makes a difference here: https://github.com/june07/nimv3/blob/main/src/sw.js#L417-L418 where again, simply had to write a workaround to get a usable devtools environment using the compat endpoint... because the js_app.html endpoint does not provide a lot of features like a browsable file tree for example see the gif... but also the info I think should be usable by third party apps/etc to have a definitive answer to which runtime is actually present. Elsewhere in backend code of linked projects, the info is also used to make determinations based on the runtime... for one, Deno's use of '/ws/' in the URL vs the absence of this path when using Node. The hack I put in place is just a regex and not as sure as depending on info. Here you can see what I mean where the compat version has the file browser while the other does not... Not trying to be a complainer and of course I don't know the full scope of things. Would love to get some feedback on this. I'm curious also if you could point to where in the DevTools code you found that changing the runtime results in the "Before/After" changes you mentioned? I could submit a PR to basically emulate what Node currently does by offering both endpoints... but just wanted to check here first. As well there's some uncertainty about my last question. Thanks. |
This is intentional as both runtimes are similar - not using
I'm not sure I understand why the other URL is needed? The code that generates the URL is here: deno/runtime/inspector_server.rs Line 408 in 2b247be
I'm not sure I understand this question either but I'll try to answer anyway: we found this empirically that using |
This partially fixes #10113, by allowing Deno to use Chrome's builtin support for debugging NodeJS processes. Longterm we'll want to add first-class Deno support in devtools & chromium.
Impact
Users are shown a significantly more relevant DevTools view when clicking
inspect
(see screenshots below) and can also useOpen dedicated DevTools for Node
which will open a DevTools window (with relevant views) that auto-attaches to newly spawned inspectable (deno) processes, which is nice.Comparison of DevTools views
This compares the view users get when they go to
chrome://inspect
and click oninspect
.Before
You'll notice this view includes web specific tabs such as Elements (DOM), Lighthouse, Application, etc...
After
This view now contains only the relevant tabs and (CPU)
Profiler
instead ofPerformance