dev: adapt to vitest@1.2 #213
Conversation
| import { isDefinitelyVitestEnv, mayBeVitestEnv } from './pure/isVitestEnv' | ||
| import { getVitestCommand, getVitestVersion, isNodeAvailable } from './pure/utils' | ||
| import { log } from './log' | ||
| export const extensionId = 'zxch3n.vitest-explorer' | ||
|
|
||
| // Copied from https://github.com/vitest-dev/vitest/blob/main/packages/vitest/src/defaults.ts | ||
| // "import { configDefaults } from 'vitest'" throws unexpected URL error |
There was a problem hiding this comment.
This statement throws "Invalid URL" error during activation (from node_modules/vite/dist/node-cjs/publicUtils.cjs. That file includes codes like below:
const { version: version$2 } = JSON.parse(fs$1.readFileSync(new URL('../../package.json', (typeof document === 'undefined' ? require('u' + 'rl').pathToFileURL(__filename).href : (_documentCurrentScript && _documentCurrentScript.src || new URL('node-cjs/publicUtils.cjs', document.baseURI).href)))).toString());It won't work in the bundled dist/extension.js. I couldn't find any solution and gave up. Are anyone knows something?
There was a problem hiding this comment.
I would expect Vite to not be bundled with this extension, maybe it can be externalized/tree-shaken?
There was a problem hiding this comment.
vitest/config doesn't actually import anything from Vite, so I'm not sure why it even gets bundled here 🤔 (Oh, now I see it re-exports mergeConfig). I think we should just update the bundler here to remove the mergeConfig API from vitest/config
There was a problem hiding this comment.
It looks like tsup/esbuild's default tree-shaking is not doing much and keeps unused require("vite"). Actually tsup supports rollup-based tree-shaking https://tsup.egoist.dev/#tree-shaking, so this config seems to work but rebuild slows down a bit.
// tsup.config.ts
import { defineConfig } from "tsup"
export default defineConfig({
entry: ["./src/extension.ts"],
external: ["vscode", "vite"],
treeshake: true,
})Personally, just inlining defaultInclude and defaultExclude is equally fine as well.
There was a problem hiding this comment.
tsup also supports plugins where we can manually remove vite import
There was a problem hiding this comment.
It is ideal to import the original config from vitest. I think it's problem of vitest bundle. new URL with relative path resolution won't work imported bundle. Anyway, can it be acceptable?
There was a problem hiding this comment.
I think it's problem of vitest bundle
it is not the problem of Vitest bundle, Vitest doesn't use new URL, this is the code from Vite, and Vite should not be bundled for the extension. To remove vite import, you can write a small esbuild plugin that strips it out
There was a problem hiding this comment.
Ok, let's keep hardcoded config for now
There was a problem hiding this comment.
Also, I wonder if require would work:
require('vitest/config', { paths: [workspaceDir] })
|
@zxch3n Hi, would you have time to see this PR? |
|
I am also interested to hear if you are still interested in maintaining the project. |
src/watch.ts
Outdated
| continue | ||
|
|
||
| if (type === 'test' && !(candidate instanceof TestCase)) | ||
| if (task.type === 'test' && !(candidate instanceof TestCase)) |
There was a problem hiding this comment.
I don't understand what's type='custom'. It should be treated as which one? It seems to be newly introduced in v1.
This PR includes an adaption to the latest vitest distribution v1.2.2 (2024-02-16)