kit icon indicating copy to clipboard operation
kit copied to clipboard

change default aliases to be more like node's subpath aliases

Open weepy opened this issue 3 years ago • 3 comments

Describe the problem

I want to run a node test runner to test parts of my application, but it doesn't understand the "$lib" type aliases. I tried the node package module-alias but it appears difficult to use with es6. Node already has support for subpath aliases, but they must begin with "#", so currently I am changing the sveltekit to use "#lib" via :

vite: {
			resolve: {
				alias: {
					'#lib': path.resolve('./src/lib')
				}
			}
		}

which is not a bad solution, but perhaps parity with node should be the default?

Describe the proposed solution

"$lib" changed to "#lib"

Alternatives considered

No response

Importance

would make my life easier

Additional Information

No response

weepy avatar Aug 08 '22 05:08 weepy

I think this should be supported via https://github.com/vitejs/vite/issues/7385 first so we don't mix Vite's aliases with Node's aliases.

bluwy avatar Aug 08 '22 06:08 bluwy

Looks like there's a PR for that one, but it's in need of a review: https://github.com/vitejs/vite/pull/7770

benmccann avatar Aug 08 '22 17:08 benmccann

Vite aliases are very nice because you can override them to provide no-ops or your own behavior for $app/navigation, $app/stores, etc. when doing things like using Storybook, testing, etc. I'm not sure whether you'd be able to do that with #app. Perhaps you could still override with a Vite alias, but I'd want to make sure.

benmccann avatar Aug 10 '22 14:08 benmccann

I'm hesitant to make this change, because it only really makes sense for $lib — not $env or $app — and it would feel weird to have #lib alongside $app/....

In general, Vite apps are going to contain stuff that isn't Node-compatible — import.meta.glob, .css imports, and all sorts of other things — so I'm not sure if there's all that much value in making a change that makes it possible for Node test runners to run in a slightly larger subset of cases (those involving $lib) than is currently the case, given that it's always going to be much less than 100%.

I think a combination of these strategies is likely to yield better results:

  • Using Vitest, or another Vite-aware test runner
  • Only unit-testing code inside lib, rather than trying to unit test code that relies on lib
  • Leaning more on integration tests than unit tests

Rich-Harris avatar Aug 25 '22 20:08 Rich-Harris

Makes sense !

Perhaps using vitest is the way to go.

Much of code inside lib at least for me but imagine many others will refer to $lib simply because of the convenience.

I’ve seen you (RH) strongly advise against using jest. Given the api is similar what do you say of vitest. !

On Thu, 25 Aug 2022 at 22:31, Rich Harris @.***> wrote:

I'm hesitant to make this change, because it only really makes sense for $lib — not $env or $app — and it would feel weird to have #lib alongside $app/....

In general, Vite apps are going to contain stuff that isn't Node-compatible — import.meta.glob, .css imports, and all sorts of other things — so I'm not sure if there's all that much value in making a change that makes it possible for Node test runners to run in a slightly larger subset of cases (those involving $lib) than is currently the case, given that it's always going to be much less than 100%.

I think a combination of these strategies is likely to yield better results:

  • Using Vitest, or another Vite-aware test runner
  • Only unit-testing code inside lib, rather than trying to unit test code that relies on lib
  • Leaning more on integration tests than unit tests

— Reply to this email directly, view it on GitHub https://github.com/sveltejs/kit/issues/5847#issuecomment-1227731420, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGNEU65M4YRHIU7G6CX3V27J3ZANCNFSM5533CMRQ . You are receiving this because you authored the thread.Message ID: @.***>

weepy avatar Aug 25 '22 21:08 weepy

I haven't really put it through its paces, but I'm very impressed with what I've seen so far of Vitest. We're considering adding it as an option when you create a new app: #5708

Will close this issue then — thanks

Rich-Harris avatar Aug 25 '22 21:08 Rich-Harris