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

Allow more flexible glob matchers for runic js/ts modules #11536

Closed
Hugos68 opened this issue May 10, 2024 · 10 comments
Closed

Allow more flexible glob matchers for runic js/ts modules #11536

Hugos68 opened this issue May 10, 2024 · 10 comments
Assignees
Milestone

Comments

@Hugos68
Copy link

Hugos68 commented May 10, 2024

Describe the problem

When you want to write runic code outside .svelte files you require to use the .svelte.js or .svelte.ts extension. This is fine and benefits compile times.

The issue with this is that it can easily conflict with other libraries/frameworks that also utilize files extensions, in my case: Vitest.
Sveltekit by default through the npm create svelte CLI adds the following matchers:

export default defineConfig({
	plugins: [sveltekit()],
	test: {
		include: ['src/**/*.{test,spec}.{js,ts}']
	}
});

This means when you require runic code inside test files you need to first rename your test file to .test.svelte.ts and then add the matcher to the vite config. Not only is this tedious but VSCode extensions like Vitest also don't understand this extra file extension which hurts the DX quite a bit for writing runic code/tests.
(Yes, you need runes inside test files because you might test a module that receives some state)

Describe the proposed solution

The solution would be that the compiler uses a more flexible matcher for compiling js/ts files, of the top of my head I came up with: *.svelte.*{js,ts}.
Here are a couple of test cases that showcase the matches:

svelte-module.ts // ignored
module-svelte.ts // ignored
svelte.module.ts // ignored
module.svelte.ts // match
module.svelte.test.ts // match
module.test.svelte.ts // match

This allows for more flexible file names that don't degrade the experience with testing. I'm sure there are more frameworks/libraries that also utilize file extensions (potentially storybook) so I wouldn't say this is an edge case but a pretty common use case.

A bonus would be if when checking the Vitest option through the CLI scaffolds these matchers.

Importance

would make my life easier

@JReinhold
Copy link

On the top of my head I can't come up with an actual use case for this in Storybook, but in theory it could be a problem that Storybook requires *.stories.ts/svelte and runes are only compiled in *.svelte.ts/*.svelte.

👍

@paoloricciuti
Copy link
Member

I wonder if this could instead be solved by providing a better initial template? For instance doing this in the vite.config

export default defineConfig({
	plugins: [sveltekit()],
	test: {
		include: ['src/**/*.{test,spec,test.svelte,spec.svelte}.{js,ts}']
	}
});

and adding

"vitest.rootConfig": ""

in a .vscode folder to point it to the root config?

@Hugos68
Copy link
Author

Hugos68 commented May 10, 2024

I wonder if this could instead be solved by providing a better initial template? For instance doing this in the vite.config

export default defineConfig({

	plugins: [sveltekit()],

	test: {

		include: ['src/**/*.{test,spec,test.svelte,spec.svelte}.{js,ts}']

	}

});

and adding

"vitest.rootConfig": ""

in a .vscode folder to point it to the root config?

This captures perfectly why it is annoying, this will differ per editor as well...

I don't think adding additional config is a great solution. So far I haven't been able to come up with any downsides with the proposed solution.

@paoloricciuti
Copy link
Member

I wonder if this could instead be solved by providing a better initial template? For instance doing this in the vite.config

export default defineConfig({

	plugins: [sveltekit()],

	test: {

		include: ['src/**/*.{test,spec,test.svelte,spec.svelte}.{js,ts}']

	}

});

and adding

"vitest.rootConfig": ""

in a .vscode folder to point it to the root config?

This captures perfectly why it is annoying, this will differ per editor as well...

I don't think adding additional config is a great solution. So far I haven't been able to come up with any downsides with the proposed solution.

Changing the config took me 1 minute and if we change the create-svelte template it will take even less (literally 0). Also the problem with the vscode extension for vitest is only for vscode 😄

@Hugos68
Copy link
Author

Hugos68 commented May 10, 2024

Because you probably have more experience with runes and svelte than most people using svelte. Any minute of hassle will prevent people from doing what they want. Like I said the proposed solution only makes it easier without any loss. I'm not arguing it's hard to setup, I'm arguing that it can definitely be made easier.

@paoloricciuti
Copy link
Member

Because you probably have more experience with runes and svelte than most people using svelte. Any minute of hassle will prevent people from doing what they want. Like I said the proposed solution only makes it easier without any loss. I'm not arguing it's hard to setup, I'm arguing that it can definitely be made easier.

I agree with you, I'm just arguing where to ease the experience. Doing it at the compiler level means extra work for the compiler (which I don't care too much) but will also mean a slightly steeper learning curve. If we change it at the compiler level there's more ways to write runic files and less consistency.

If we modify the template you still have to setup nothing because it's already setupped.

@Hugos68
Copy link
Author

Hugos68 commented May 10, 2024

Because you probably have more experience with runes and svelte than most people using svelte. Any minute of hassle will prevent people from doing what they want. Like I said the proposed solution only makes it easier without any loss. I'm not arguing it's hard to setup, I'm arguing that it can definitely be made easier.

I agree with you, I'm just arguing where to ease the experience. Doing it at the compiler level means extra work for the compiler (which I don't care too much) but will also mean a slightly steeper learning curve. If we change it at the compiler level there's more ways to write runic files and less consistency.

If we modify the template you still have to setup nothing because it's already setupped.

I wouldn't say this is extra work for the compiler, globbing a few more patterns, since it has to check every filename for the pattern anyways, it just checks agaisnt a more flexible pattern. I do agree that the learning curve might steepen a bit but I'd say keep telling folks to use .svelte.js and just add a note: the compiler actually matches x and y too in case you need it.

@paoloricciuti
Copy link
Member

Because you probably have more experience with runes and svelte than most people using svelte. Any minute of hassle will prevent people from doing what they want. Like I said the proposed solution only makes it easier without any loss. I'm not arguing it's hard to setup, I'm arguing that it can definitely be made easier.

I agree with you, I'm just arguing where to ease the experience. Doing it at the compiler level means extra work for the compiler (which I don't care too much) but will also mean a slightly steeper learning curve. If we change it at the compiler level there's more ways to write runic files and less consistency.
If we modify the template you still have to setup nothing because it's already setupped.

I wouldn't say this is extra work for the compiler, globbing a few more patterns, since it has to check every filename for the pattern anyways, it just checks agaisnt a more flexible pattern. I do agree that the learning curve might steepen a bit but I'd say keep telling folks to use .svelte.js and just add a note: the compiler actually matches x and y too in case you need it.

I've yet to see the disadvantage of changing the template tho.

@Hugos68
Copy link
Author

Hugos68 commented May 10, 2024

I've yet to see the disadvantage of changing the template tho.

Agreed, see my original issue:

A bonus would be if when checking the Vitest option through the CLI scaffolds these matchers.

@Hugos68 Hugos68 changed the title Allow more flexible glob matches for runic js/ts modules Allow more flexible glob matchers for runic js/ts modules May 10, 2024
@dummdidumm dummdidumm added this to the 5.0 milestone May 10, 2024
@dummdidumm
Copy link
Member

vite-plugin-svelte allows infix notation now. Same needs to happen for rollup-plugin-svelte and svelte-loader probably.

@dummdidumm dummdidumm self-assigned this Jun 5, 2024
dummdidumm added a commit to sveltejs/svelte-loader that referenced this issue Jun 6, 2024
dummdidumm added a commit to sveltejs/rollup-plugin-svelte that referenced this issue Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants