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.

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

Goto Definition is broken #53

Closed
Gert-JanPeeters opened this issue Oct 23, 2023 · 6 comments · Fixed by #59
Closed

Goto Definition is broken #53

Gert-JanPeeters opened this issue Oct 23, 2023 · 6 comments · Fixed by #59
Labels
bug Something isn't working

Comments

@Gert-JanPeeters
Copy link
Contributor

I am using neovim, when I try to "Goto Definition", I get the following error:

Error executing vim.schedule lua callback: ...l/Cellar/neovim/0.9.4/share/nvim/runtime/lua/vim/uri.lua:107: URI must contain a scheme: /usr/local/var/www/sticket/app/javascript/controllers/show_controller.js
stack traceback:
        [C]: in function 'assert'
        ...l/Cellar/neovim/0.9.4/share/nvim/runtime/lua/vim/uri.lua:107: in function 'uri_to_fname'
        ...l/Cellar/neovim/0.9.4/share/nvim/runtime/lua/vim/uri.lua:128: in function 'uri_to_bufnr'
        ...lar/neovim/0.9.4/share/nvim/runtime/lua/vim/lsp/util.lua:1127: in function 'jump_to_location'
        ...neovim/0.9.4/share/nvim/runtime/lua/vim/lsp/handlers.lua:412: in function 'handler'
        ...l/Cellar/neovim/0.9.4/share/nvim/runtime/lua/vim/lsp.lua:1393: in function ''
        vim/_editor.lua: in function <vim/_editor.lua:0>

I think neovim is expecting a URI with a scheme, e.g. file://usr/local/var/www/sticket/app/javascript/controllers/show_controller.js instead of /usr/local/var/www/sticket/app/javascript/controllers/show_controller.js

@marcoroth
Copy link
Owner

Thanks for reporting this, @Gert-JanPeeters!

I wonder if VSCode would still accept it with the file:// scheme, so we could use the same code for both.

Looking at this line here, I'm wondering why I opted for stripping it 🤔

this.project = new Project(this.settings.projectPath.replace("file://", ""))

@Gert-JanPeeters
Copy link
Contributor Author

Gert-JanPeeters commented Oct 23, 2023

I went searching a bit and found a similar issue:

NomicFoundation/hardhat-vscode#355

I think that VSCode can deal with both but Neovim is forcing LSPs to use an URI with a scheme. Not really sure which scheme it has to be used but I am guessing file://.

@marcoroth
Copy link
Owner

I guess in that case we should be able to use always use file:// then 👍🏼

@Gert-JanPeeters
Copy link
Contributor Author

I have investigated a bit further.

I tried changing this line:

this.project = new Project(this.settings.projectPath.replace("file://", ""))

to

this.project = new Project(this.settings.projectPath) 

But then things break. For example, stimulus-lsp cannot find the controllers anymore.

However, if I change:

const locations = controllers.map((controller) => Location.create(controller.path, Range.create(0, 0, 0, 0)))

to:

const locations = controllers.map((controller) => Location.create(`file://${controller.path}`, Range.create(0, 0, 0, 0))) 

The Goto definition works in neovim. However, I don't think we should make the change here.

If I am correct then it is stimulus-parser that provides the paths to the locations? Maybe I should make an issue / pull request there to send the controller.path with the right scheme?

@marcoroth
Copy link
Owner

Thanks for investigating!

I think we should rather do it here in the repo, since this is more LSP specific. The Stimulus Parser could be used outside of an LSP context, which is why I think it doesn't really make sense to use the file:// scheme in the stimulus-parser project.

@jondkinney
Copy link

This is probably not the right place to share this, but I ended up here after some googling about not being able to go to definition in javascript ESM files in neovim in a Rails project.

The files I'm trying to navigate exist in a user defined pinned folder in the app/javascript/custom using local modules with import maps in Rails like this: https://stackoverflow.com/questions/70548841/how-to-add-custom-js-file-to-new-rails-7-project (you have to piece things together a bit from this post).

Here's what worked for me.

config/importmap.rb 👇

# Pin npm packages by running ./bin/importmap

pin "application"
pin "@hotwired/turbo-rails", to: "turbo.min.js"
pin "@hotwired/stimulus", to: "stimulus.min.js"
pin "@hotwired/stimulus-loading", to: "stimulus-loading.js"
pin_all_from "app/javascript/controllers", under: "controllers"
pin_all_from "app/javascript/custom", under: "custom" # this is the new/important line to load our custom folder

application.js 👇

import "custom/trix_date_extension";

app/javascript/custom/trix_date_extension.js 👇

import { FormManager } from "custom/form_manager";
import { Logger } from "custom/logger";

export class TrixDateExtension {
  //... lots of code

  // Custom button handler
  document.addEventListener("click", async (event) => {
    const dateButton = event.target.closest(
      "[data-trix-action='insertDatePlaceholder']",
    );
    if (dateButton) {
      event.preventDefault();
      try {
        await this.#formManager.openTaskDatePopup(dateButton);
      } catch (error) {
        Logger.logError(error, "insertDatePlaceholder");
      }
    }
  });  
}

document.addEventListener("DOMContentLoaded", function () {
  console.log("Loading Trix Date Extension...");
  new TrixDateExtension();
});

app/javascript/custom/form_manager.js 👇

import { DOMHelper } from "custom/dom_helper";
import { Logger } from "custom/logger";

export class FormManager {
  #editor;
  #taskManager;

  constructor(editor, taskManager) {
    this.#editor = editor;
    this.#taskManager = taskManager;
    this.lastFormData = {
      taskName: "",
      dateTag: "",
      dateTime: "",
    };
  }

  async openTaskDatePopup(button) {
    try {
      const result = await this.promptForTaskDateDetails(
        button,
        this.getNextTaskDateTagInt(),
      );

      if (!result) {
        return;
      }

      const { dateTag, taskName, date } = result;
      await this.#taskManager.createTask(dateTag, date, taskName);
    } catch (error) {
      Logger.logError(error, "Error handling date placeholder");
      alert("There was an error creating the date placeholder and task.");
    }
  }

  // ... more code
}

Because of the way the import statements have to be setup for local modules with import { Klass } from "custom/klass" neovim's typical LSP Servers won't go to definition on something like the Logger.logError(... method in the code above.

But if you add this jsconfig.json file at the root of your Rails app

{
  "compilerOptions": {
    "baseUrl": "./app/javascript",
    "paths": {
      "controllers/*": [
        "controllers/*"
      ],
      "custom/*": [
        "custom/*"
      ]
    }
  },
  "include": [
    "app/javascript/**/*"
  ]
}

Then it will allow for navigating to those definitions again.

google keyword help: go-to-definition, go_to_definition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants