language-tools icon indicating copy to clipboard operation
language-tools copied to clipboard

Add support for dynamicCompileOptions in VSCode

Open bfanger opened this issue 1 year ago • 5 comments

Description

The vite plugin allows setting compileOptions based on filename which is useful for settings like runes.

At the moment the vitePlugin.dynamicCompileOptions is ignored by the language-server and warnings like:

`count` is updated, but is not declared with `$state(...)`.  Changing its value will not correctly trigger updates

are not visible in the editor when the { runes: true } is set by the dynamicCompileOptions function and no runes are used yet.

Proposed solution

Take the dynamicCompileOptions into account and use the same final compileOptions as is used for real.

Alternatives

Not using dynamicCompileOptions and relying on a preprocessor to enable runes mode.

Additional Information, eg. Screenshots

https://github.com/sveltejs/svelte/issues/12327

bfanger avatar Jul 07 '24 11:07 bfanger

The option passed into vite plugin svelte is not visible from outside so it won't be possible to read that. It will need to be option in svelte.config.js. But l don't think adding an option solely for work around an issue make much sense.

Probably better you just add more precise check in dynamicCompileOptions to exclude rune mode packages. The language server also supports nested svelte.config.js so you can also add one in the nested directory only for the editor.

jasonlyu123 avatar Jul 07 '24 12:07 jasonlyu123

The dynamicCompileOptions are defined in svelte.config.js:

// svelte.config.js
export {
  compilerOptions: { runes: true },
  vitePlugin: {
    dynamicCompileOptions({ filename }) {
      if (filename.includes("node_modules")) {
        return { runes: undefined };
      }
    },
  },

And the dynamicCompileOptions wants:

dynamicCompileOptions?: (data: {
  filename: string;
  code: string;
  compileOptions: Partial<CompileOptions>;
}) => Promise<Partial<CompileOptions> | void> | Partial<CompileOptions> | void;

which are all available in the language server.

Nested svelte.config.js may work in de language server, but they are ignored in SvelteKit (it was one of my suggestions in 12327 which was quickly marked as non-planned but the related https://github.com/sveltejs/svelte/issues/11523 is still open)

bfanger avatar Jul 07 '24 12:07 bfanger

We also don't support the vitePlugin option in the svelte.config.js. So it'll be the same as adding an dedicated option. It might even be slightly worse than a new top-level option since there might be people adding option there expecting it will only be read by vite plugin svelte.

About nested svelte.config.js. I know it's ignoed in sveltekit so it can be used as temporary workaround since only the editor will read it. But a more precise check in dynamicCompileOptions to only enable rune:true for rune mode libary will make more sense and give you more control.

jasonlyu123 avatar Jul 07 '24 12:07 jasonlyu123

I added dynamicCompileOptions to vite-plugin-svelte a while ago as an experimental option and it was promoted to a stable option since. Because it only works with vite-plugin-svelte it's not on the top-level of svelte.config.js

IF we add it to the top, then it would have to be supported by svelte-loader, rollup-plugin-svelte etc. too.

dominikg avatar Jul 07 '24 13:07 dominikg

We'll likely build this into Svelte itself in the sense of having a svelte/config or @sveltejs/config and enhancing svelte.config.js so that compilerOptions can also take a function similar to dynamicCompileOptions. Once these steps are implemented we should be able to support it in here aswell.

dummdidumm avatar Jul 30 '24 10:07 dummdidumm