mxsdev / nvim-dap-vscode-js

nvim-dap adapter for vscode-js-debug
271 stars 27 forks source link

Adds support for the `node` adapter #13

Open eastwood opened 1 year ago

eastwood commented 1 year ago

As of July 2022, the pwa-node adapter became the default node adapter and is now available under the node type by default.

In this PR, I've updated the config filter to support the launch configuration containing the node type. I've left the existing pwa-node type for backwards compatibility until such time that the pwa-node adapter will become officially deprecated.

Essentially, this is a name change - so I've copied the tests across to make sure that we can catch any funny business if the two adapter diverge.

I'm very new to lua, so have used your workflow to run test cases, and change requests are encouraged and welcome :)

sspatari commented 1 year ago

@mxsdev can you look at this?

mxsdev commented 1 year ago

Hi! Thank you so much for the pr and for being open to change requests :)

If I understand correctly, this is in reference to microsoft/vscode-js-debug#1305? If so, I believe those changes apply only at the external configuration level (think VSCode's launch.json).

Correct me if I am wrong, but I think that your tests still use the old launch_config - so it appears exactly identical to the old tests. Trying to run with the type as node in my tests results in an error, which is caused by this line of the debugger.

Basically, there is an extra step between a client like VSCode and the internal debugger, which replaces node with pwa-node (see here). I'm pretty sure this layer of indirection happens client-side, not server-side.

Since there is the possibility of pwa-* being removed internally, here is my proposal going forward for this project:

Then if down the road pwa_node gets totally wiped internally and it is replaced by node, we can just remove the alias and forbid pwa_node for all versions of vscode-js-debug not sufficiently new.

Let me know how you feel about this approach. You say you're not super familiar with lua so I'm happy to implement it myself, just let me know 👍

dieguit commented 1 year ago

Hello! Thanks @mxsdev for looking into this. I have an issue with this node vs pwa-node naming, so I'm wondering how this PR will proceed. Basically, I am using a launch.json file that has stuff like

    {
      "name": "some-service",
      "type": "node",
      "request": "attach",
      "port": 9000
    },

For this to work, after adding the pwa-node adapter with this plugin I need to:

  1. Add another entry (or change the existing one so it's
    {
      "name": "some-service",
      "type": "pwa-node",
      "request": "attach",
      "port": 9000
    },
  1. Update the launchjs to be:
require('dap.ext.vscode').load_launchjs(nil, {
  ['pwa-node'] = {
    'javascript',
    'typescript',
  },
  ['node'] = {
    'javascript',
    'typescript',
  },
})

I'm not sure if I'm missing something or this could be done in a better way, but i'd be nice if we were able tu use just node in the launch.json entry

80avin commented 4 months ago

@dieguit You can do it like this

local ok, filetypes = pcall(require, 'mason-nvim-dap.mappings.filetypes')
require("dap.ext.vscode").load_launchjs(nil, ok and filetypes or nil)
80avin commented 4 months ago

@eastwood Is there any progress on this ?

I don't want to do duplicate efforts but if this PR is abandoned, I'll prefer to work on another PR

80avin commented 4 months ago

Hi @mxsdev , I'm thinking of completing this PR and need one clarification on the suggestions you've given.

Allow node, chrome, msedge, and extensionHost as options, and alias them to the pwa-* equivalents internally

Is it correct or otherwise ? I think logical is to alias the pwa-* equivalents to node, chrome, msedge and extensionHost equivalents. i.e. If user provides "pwa-node", it should be mapped to "node" and used so.