mfussenegger / nvim-dap

Debug Adapter Protocol client implementation for Neovim
GNU General Public License v3.0
5.46k stars 194 forks source link

Support type=node instead of type=pwa-node for vscode-js-debug #1104

Closed Otard95 closed 10 months ago

Otard95 commented 10 months ago

Debug adapter definition and debug configuration

Not sure if it's entirely correct to call this a bug, but I feel like it's more than just a discussion.

Adapter installed via mason-nvim-dap.

  require 'mason-nvim-dap'.setup {
    ensure_installed = {
      'js',
      'php'
    },
  }

Configured like:

  require 'dap.ext.vscode'.load_launchjs('.vscode/launch.json', {
    node = { 'typescript', 'javascript' },
  })
  dap.adapters["node"] = {
    type = 'server',
    port = 8123,
    executable = {
      command = 'js-debug-adapter',
      args = { '8123' },
    },
    options = {
      max_retries = 40,
    },
  }

Debug adapter version

No response

Steps to Reproduce

Any basic configuration should yield the same result.

Expected Result

I'd like to see "type": "node", in launch.json work. Considering that VS Code expects this, though not required.

image

Actual Result

The debugger fails with Error: Unknown config.

Considering VS Code want's us to use node instead of pwa-node maybe require 'dap.ext.vscode'.load_launchjs(...) should change the node type to pwa-node. Of course this creates a little bit of a hidden disconnect between the type in the configuration and adapter, but considering VS Code itself urges us not to use pwa-node, it might be the better option?

Otard95 commented 10 months ago

To be clear. I've gotten this to work with pwa-node in my launch.json and everything else configured accordingly. I'd just expect that the node type would work as it's what VS Code tells us to use.

mfussenegger commented 10 months ago

As far as I can tell the pwa prefixes are still required in the launch configuration:

https://github.com/microsoft/vscode-js-debug/blob/e51f7cc6ccb3f44b328e386f28109f305bc5741d/src/common/contributionUtils.ts#L65-L71

I suspect their client extension might map node to pwa-node somewhere to make it work with just node. I'm not adding debug adapter specific logic to nvim-dap. This would have to change in the adapter

Otard95 commented 10 months ago

I agree, you shouldn't. I may have been a bit unclear in my original description. I'm not suggesting a change to how nvim-dap talks to the adapter.

But from what I understand load_launchjs is a util to allow you to use a VS Code launch configuration with nvim-dap, and if so would it not make sense to have that util, and just that util, handle a configuration in the format VS Code wants it? That is, it should translate a launch.json to what the debug adapter expects?

Of course, I may be misunderstanding the role of load_launchjs, and in which case I already have a solution that works for my use-case without needing load_launchjs, and without diverging from how VS Code says the launch.json should be. I just wanted to forward an issue I encountered that seemed counter to how I thought load_launchjs was supposed to work.

For reference, this is how I handled it in my config. Admittedly it's not perfect, I've still not factored out the need for a custom entry in the launch.json for defining what filetypes it should attach to. But it does translate the node type into pwa-node, from what VS Code expects into what the adapter expects.

mfussenegger commented 10 months ago

It's not just about the launch.json, their options documentation also mentions node instead of pwa-node, but if that's used in the launch request payload it doesn't work.

What you could do is try something like this:

local dap = require("dap")
local node_adapter = {
  type = "server",
  host = "localhost",
  port = "${port}",
  executable = {
    command = "node",
    args = {"/path/to/js-debug/src/dapDebugServer.js", "${port}"},
  },
  enrich_config = function(conf, on_config)
    if not vim.startswith(conf.type, "pwa-") then
      local config = vim.deepcopy(conf)
      config.type = "pwa-" .. config.type
      on_config(config)
    else
      on_config(conf)
    end
  end
}
dap.adapters["node"] = node_adapter
dap.adapters["pwa-node"] = node_adapter

You need both, the node and pwa-node adapter definition - and the node definition changes the type.

But I still think node should be supported as type in the adapter, if that's what they document

Otard95 commented 10 months ago

Honestly, I find that document a little confusing, as the word "type" is not mentioned once, but I suppose maybe the heading is referring to the type and request? In any case, I understand not wanting to deal with this, and your suggested configuration does indeed work! Thanks for your help and great plugin!