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

Package.json detection fails if the package.json is not in the root #272

Open
entropitor opened this issue Jan 31, 2018 · 25 comments · May be fixed by #729
Open

Package.json detection fails if the package.json is not in the root #272

entropitor opened this issue Jan 31, 2018 · 25 comments · May be fixed by #729

Comments

@entropitor
Copy link

E.g. I'm working in a mono-repo with multiple package.json's in my repository, vim-test cannot find the other ones. vim-lsp for examples can cope with this using some helper functions

\ 'root_uri':{server_info->lsp#utils#path_to_uri(lsp#utils#find_nearest_parent_file_directory(lsp#utils#get_buffer_path(), '.flowconfig'))},

It would be nice if we could also configure this for vim-test

@codeinabox
Copy link
Collaborator

I guess it could be possible to have an option to manually override the path it checks for package.json but in this case are you staying there are multiple ones, and I am guessing each of them might have a different test runner?

I need to check with @janko-m but I believe the scope of this plugin is just for it running in the root directory of a single project

@janko
Copy link
Member

janko commented Jan 31, 2018

I'm not sure I understand yet what exactly vim-test needs to do here, but if any feature request that would otherwise require overriding the :Test* commands I would consider to be in the scope of vim-test.

If this use case is too specific, it might also be worthwhile to add some Vim auto-commands to allow people to change some things dynamically.

@entropitor
Copy link
Author

@codeinabox The way vim-lsp does things is by determining the command to run based on which buffer you have open. So potentially that could mean multiple test runners, however it's always determined which one it should be (by just going up until you find a package.json)

@codeinabox
Copy link
Collaborator

Yes but there will be package.json in people's vendor directories

@entropitor
Copy link
Author

@codeinabox But that is not in the path to the file, is it? It's normally in a node_modules directory next to the app, I cannot imagine any situation in which a vendor's package.json would be found anywhere on the path from the file to the root directory

@codeinabox
Copy link
Collaborator

Now I am working on a project with mono repo I can see the problem but I don't see this a problem specifically with vim-test but any plugin that assumes the current working directory is the project directory. Keen to find a solution but ideally one that works across multiple plugins and not just vim-test.

@entropitor
Copy link
Author

@codeinabox Given this project already uses projectionist, maybe it should use the result of the function call call projectionist#path() as the project dir for a file?

@codeinabox
Copy link
Collaborator

Interesting, so projectionist let's you define multiple projects and call projectionist#path() will give you the project root path for that file?

@entropitor
Copy link
Author

Yes, exactly!

@codeinabox
Copy link
Collaborator

@codeinabox Given this project already uses projectionist, maybe it should use the result of the function call call projectionist#path() as the project dir for a file?

@janko do think using call projectionist#path() to determine the project root is something we should implement for just package.json detection or would it be worth moving into the base functionality of this plugin?

@janko
Copy link
Member

janko commented Jun 27, 2019

So, this will only work if the user is using projectionist.vim and has it configured? I'm ok with that. If the user doesn't use projectionist.vim, we can just fall back to the current behaviour.

@codeinabox
Copy link
Collaborator

@entropitor what does your .projections.json file look like and is it located at the root of the mono repo?

@entropitor
Copy link
Author

I just have one in every "subrepo" of the monorepo, it's a standard one, nothing special. And I do believe it makes a lot of sense to use that path over the current dir if the user is using projectionist and there is a file for the current project

@codeinabox
Copy link
Collaborator

I do like this idea and I'm keen to help out implementing projectionist support though I won't have the chance to do so till mid August. Feel free to raise a PR in the interim

@entropitor
Copy link
Author

Do you have any pointers on how to start on this?

@JoosepAlviste
Copy link

I commented on #372 but I think it makes more sense to discuss the problem here (maybe even close the other issue?) Here's my idea not requiring projectionist but just recursively searching for the package.json:

I have a similar case where my project is split into an api/ and an app/. Something like this:

/my-project/
-- api/
-- -- package.json
-- app/
-- -- src/components/
-- -- -- Foo.js
-- -- -- Foo.test.js
-- -- package.json
-- package.json  < includes general dependencies like husky and utility scripts

This means that this plugin always wants to use the /my-project/package.json file and not the package.json closest to the file I'm editing. It seems logical to me that the package.json would be searched for recursively from the file I'm editing. This way it should always be correct.

I think this is how most plugins that want to use node_modules/ work but I'm not 100% sure on that. It is definitely how I would expect it to work though. Any ideas?

@JoosepAlviste
Copy link

JoosepAlviste commented Jul 27, 2019

I'm working on this a little bit in https://github.com/JoosepAlviste/vim-test/tree/non-root-package-json but there's a small problem with the solution.

Detecting the correct package.json and node_modules/.bin/jest works fine, but since the project root is not in the same folder as the package.json file, Jest will be executed from the root folder. This means that Jest configuration from jest.config.js or the correct package.json is not read.

For my example above:

/my-project/ <- 3. But this is the current working directory so Jest is executed in this directory
-- api/
-- -- package.json
-- app/
-- -- src/components/
-- -- -- Foo.js
-- -- -- Foo.test.js <- 1. I execute the tests in this file
-- -- package.json <- 2. This gets detected correctly and the correct Jest executable is used
-- -- jest.config.js <- 4. And this config file does not get read
-- package.json

I have two ideas on how to solve this (neither seems "good enough" though):

  1. Temporarily switch the working directory to the directory where the package.json is found and after running tests, swap it back.
  2. Also look for the jest.config.js file in the file tree and add this path to the Jest CLI arguments. However, this file is not necessarily next to the package.json and can not even exist at all.

What do you think is a reasonable solution here? This is probably also relevant for the other Javascript test runners so it would be nice to have a generic solution.

I guess the most reasonable option is to always cd to the correct project root before running tests:

  1. User runs TestFile
  2. Detect project root based on a bunch of files that can be configured by the user (.git/, .projectionist.json, package.json, etc.)
  3. cd to the project root
  4. Run the tests
  5. cd back to the previous working directory

@codeinabox
Copy link
Collaborator

@JoosepAlviste are you using projectionist too? I found for me working with mono repos the simplest solution is have each sub project in a tab and in Neovim using tcd to set the working directory of each tab which means my tests and also other plug-ins such as Neomake and Neoformat work fine.

@janko
Copy link
Member

janko commented Jul 28, 2019

Just wanted to note that regardless of projectionist, we should still have an implementation that doesn’t rely on it.

@JoosepAlviste
Copy link

@JoosepAlviste are you using projectionist too? I found for me working with mono repos the simplest solution is have each sub project in a tab and in Neovim using tcd to set the working directory of each tab which means my tests and also other plug-ins such as Neomake and Neoformat work fine.

I recently started using projectionist but I've only used the alternative files feature and haven't done much with .projectionist.json.

I've also tried to just manually cd into the relevant project but I would really like it to be more automatic and magical. For now, something like the following seems to work fine for me and my set-up, but it could be converted to a more generic solution:

let g:root_markers = ['package.json', '.git/']
function! s:RunVimTest(cmd)
    " I guess this part could be replaced by projectionist#project_root
    for marker in g:root_markers
        let marker_file = findfile(marker, expand('%:p:h') . ';')
        if strlen(marker_file) > 0
            let g:test#project_root = fnamemodify(marker_file, ":p:h")
            break
        endif
        let marker_dir = finddir(marker, expand('%:p:h') . ';')
        if strlen(marker_dir) > 0
            let g:test#project_root = fnamemodify(marker_dir, ":p:h")
            break
        endif
    endfor

    execute a:cmd
endfunction

nnoremap <leader>tf :call <SID>RunVimTest('TestFile')<cr>
nnoremap <leader>tn :call <SID>RunVimTest('TestNearest')<cr>
nnoremap <leader>tf :call <SID>RunVimTest('TestSuite')<cr>
nnoremap <leader>ts :call <SID>RunVimTest('TestFile')<cr>
nnoremap <leader>tl :call <SID>RunVimTest('TestLast')<cr>
nnoremap <leader>tv :call <SID>RunVimTest('TestVisit')<cr>

@codeinabox
Copy link
Collaborator

Keen to revisit and solve this issue, it would be quite easy add some unit tests around it because we just need a scenario when you are one directory below. A couple of questions though:

  1. If you're working with a mono-repo and you want to run a test in a sub project should this plug-in be able to recognise the root of a sub project?
  2. Should the working directory be temporarily changed to the root of a sub project or should it be passed to the the specific test runner? A lot of the runners are configured to use the executables in node_modules/.bin and this is all based on the working directory being the project root

@codeinabox
Copy link
Collaborator

codeinabox commented Jun 30, 2020

I've been thinking about this and now feel that handling mono repositories is perhaps beyond the scope of this plug-in and actually warrants its own plug-in for the following reasons:

  1. Many of the plug-ins I use, take Neoformat for example, do not behave properly if the working directory is not the root directory of the sub project
  2. I get around this by opening each sub project in a different tab and correspondingly setting its working directory
  3. If this behaviour could be automated in a separate plug-in not only would it fix issues in vim-test but other plug-ins not designed with a mono repo in mind would work. For example vim-rspec also has issues with mono repos
  4. It would also reduce the complexity vim-test as all the existing logic and tests are based on the assumption that the working directory is the root of the project not the repository

What are your thoughts @janko and @willfish? I stumbled upon vim-rooter which could perhaps be configured to automatically changed working directories when working on a mono repo

@fnune
Copy link

fnune commented Nov 24, 2020

I've very naively added the snippet above shared by @JoosepAlviste to the before_run hook and my monorepo is working just fine:

https://github.com/fnune/vim-test/blob/master/autoload/test.vim#L161-L177

@lynndylanhurley
Copy link

lynndylanhurley commented Dec 28, 2020

Thanks @fnune + @JoosepAlviste. That solution worked for me as well 👍

@lynndylanhurley
Copy link

lynndylanhurley commented Jul 5, 2023

Finally got around to updating @JoosepAlviste's solution for my nvim config. Here's my updated lua config using lazy.nvim:

-- Find project root using root markers
local root_markers = {'Gemfile', 'package.json', '.git/'}
function _G.RunVimTest(cmd_name)
  return function()
    for _, marker in ipairs(root_markers) do
      local marker_file = vim.fn.findfile(marker, vim.fn.expand('%:p:h') .. ';')
      if #marker_file > 0 then
        vim.g['test#project_root'] = vim.fn.fnamemodify(marker_file, ":p:h")
        break
      end
      local marker_dir = vim.fn.finddir(marker, vim.fn.expand('%:p:h') .. ';')
      if #marker_dir > 0 then
        vim.g['test#project_root'] = vim.fn.fnamemodify(marker_dir, ":p:h")
        break
      end
    end

    vim.cmd(':' .. cmd_name)
  end
end

return {
  'vim-test/vim-test',
  dependencies = { { 'kassio/neoterm' } },
  config = function()
    vim.g['test#strategy'] = 'neoterm'
    vim.g['test#neoterm#term_position'] = 'vert'
    vim.g['test#preserve_screen'] = 1
    vim.g['test#javascript#runner'] = 'vitest'
    vim.g.neoterm_shell = 'zsh'
    vim.g.neoterm_automap_keys = ''
    vim.g.neoterm_default_mod = 'vertical'
  end,
  keys = {
    { '<leader>rn', RunVimTest('TestNearest'), desc = "Run nearest test" },
    { '<leader>rt', RunVimTest('TestSuite'), desc = "Run the nearest test suite" },
    { '<leader>rf', RunVimTest('TestFile'), desc = "Run all tests in the current file" },
    { '<leader>rr', RunVimTest('TestLast'), desc = "Run last test again" }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants