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

Issue sourcecodedir always overriding vitest root configuration #334

Open
2 tasks done
cgaube opened this issue Jan 12, 2023 · 1 comment
Open
2 tasks done

Issue sourcecodedir always overriding vitest root configuration #334

cgaube opened this issue Jan 12, 2023 · 1 comment
Labels
help wanted Extra attention is needed

Comments

@cgaube
Copy link

cgaube commented Jan 12, 2023

  • I have tried upgrading by running bundle update vite_ruby.
  • I have read the troubleshooting section before opening an issue.

Description 📖

vite-plugin-ruby sets the vite server root configuration https://vitejs.dev/config/shared-options.html#root from the configuration found in the json file sourceCodeDir

sourcecodedir is used for the aliases @ and ~ but also to fetch the list of assets/files to watch for rebuild.
By default it is set to app/frontend

Because the plugin make it so vite.root = 'app/frontend' any paths sent via option to vite have to be relative to that folder
This leads to issue with vitest
with reporters paths but also include / excludes paths

npx vitest --reporter=./path/to/reporter.js  

in that example ./path/to/reporter.js is relative to sourcecodedir

Why is it an issue ?

In jetbrains IDE (like rubymine) when executing a vitest bin the IDE will send the reporter path relative to the root of the project
which does not match the sourcecodedir and the IDE has no way (i dont think) to get that path value and set the right path for their custom reporters.

e.g

vitest --reporter=../../../../Applications/RubyMine.app/Contents/plugins/javascript-impl/helpers/vitest-intellij/node_modules/vitest-intellij-reporter-safe.js

Screenshot 2023-01-11 at 15 58 20

because root is now app/frontend the correct path for that reporter should be

../../../../../../Applications/RubyMine.app/Contents/plugins/javascript-impl/helpers/vitest-intellij/node_modules/vitest-intellij-safe.js
(notice the extra ../../)

How can we make it work ?

a) we can use the vitest root configuration , unfortunately because it is configured via vite plugin and vitest plugin is executed BEFORE the ruby vite plugin the root configuration is always overridden by sourcecodedir

b) the project can use ./ as value for sourcecodedir
-> problem ? bin/vite executable will take forever to execute because it now has to scan ALL files in your project in order to watch the files and trigger an automatic build of the asset on file change!
-> other problem is all the fancy aliases @ and ~ now kindof pointless.

c) We could make it so ruby plugin does not set the root configuration on mode = test
-> problem this will force people to specify a root in vite.config.js and not rely on sourcecodedir

d) And last solution which I ended up doing but is quite hacky, is to create a custom fixViteTestPlugin that will change back the root value to the one set by vitetest only when running tests.

const fixVitestPlugin = () => [
  {
    name: 'vitest-fix',
    config: (userConfig) => (userConfig.mode == 'test' ? { root: userConfig.test.root } : {}),
  },
];

export default defineConfig(({ mode }) => {
  return {
    server:  { ... }
    resolve: { ... }
    plugins: [rubyPlugin(), fixVitestPlugin()],
    ...
    test: {
      root: path.resolve(__dirname, './'),
      ...
    }
 } 

That way i can have best of both world. using sourcecode dir for app/frontend and all my test are in ./specs and rubymine works

That is it for me. I am not too sure in what project this should be fixed, ruby vite ? vitest ? or just rubymine.
Open to suggestion

Vite Ruby Info

Run bin/rake vite:info and provide the output:

bin/vite present?: true
vite_ruby: 3.2.13
vite_rails: 3.0.13
rails: 7.0.4
node: v18.12.1
npm: 8.19.2
yarn: 1.22.19
pnpm: 7.24.3
ruby: ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]
@cgaube cgaube added the bug: pending triage Something doesn't seem to be working, but hasn't been verified label Jan 12, 2023
@ElMassimo
Copy link
Owner

ElMassimo commented Mar 16, 2023

Hi Christophe!

I think the most elegant way to fix this, is for Vitest to allow resolving reporter paths against process.cwd(), as a fallback.

Additionally, allowing to explicitly pass a root to Vitest that takes priority over Vite's root would solve the problem with include and exclude, similar to this fix.


For context, here are similar problems and fixes in other plugins:

@ElMassimo ElMassimo added help wanted Extra attention is needed and removed bug: pending triage Something doesn't seem to be working, but hasn't been verified labels Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants