[feat] allow users to override IllegalModuleGuard default behaviour

[magne4000]

Closes #6700.

Allows libs like telefunc (which transforms server code when used client side) to tell sveltekit that it can safely import files otherwise considered as server side on client side.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: 5d9fffa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[brillout]

Author of Telefunc here. Let me know if there is anything we can do on our side to make progress on this PR.

Very much looking forward to be able to use SvelteKit with Telefunc.

[benmccann]

this seems like a workaround for some other bug. I'd expect the telefunc vite plugin to remove the private stuff on the client and the sveltekit vite plugin to use the version provided by the telefunc plugin. I'm not sure where that's breaking down, but hopefully we can figure that out

[magne4000]

Not sure this is entirely a bug, because:

• telefunc generates different code when isSsr=false
• sveltekit executes its dependency check when isSsr=true.

I understand now what @tcc-sejohnson meant

SvelteKit has no way of knowing that you're "not really" importing it on the client

Indeed, sveltekit has no way to know that telefunc manipulates code when isSsr=false, so the error message is understandable.

I tried many alternative solutions on Telefunc side, but none were satisfactory. It would require vitejs/vite#6810 to be solved for me to test other solutions.

But I still think that a solution on sveltekit side is the way to go. Telefunc doesn't do anything that vite does not allow. On the other hand, sveltekit dependency checks makes assumptions (which are fine 99% of the time) (in this case the assumption is that we have the same dependency tree on server and on client). It seems reasonable to think that it should offer a way to circumvent this check for cases like this one.

PS: Searching for alternative solutions, I stumbled upon Rollup Inter-plugin communication. This seems to be a better approach to handle this PR.

[benmccann]

I agree telefunc is doing nothing wrong here. I'm just not sure about this solution

sveltekit executes its dependency check when isSsr=true.

Yeah, I just realized this too. It seems problematic. I'd hope there's a way we can run the dependency check against the client-side code instead of against the server-side code since what we're worried about is secrets being imported into the client-side code

[brillout]

I'd hope there's a way we can run the dependency check against the client-side code instead of against the server-side code since what we're worried about is secrets being imported into the client-side code

Yea that'd be perfect. No need for this PR then 👌.

It's also the better approach, regardless of Telefunc: a Vite plugin may inject unsafe code into the client-side and the current approach won't catch this.

[Rich-Harris]

#7487 does not fix this issue. Reopening

[benmccann]

If we were going to move forward with this, the following two documentation files would need to be updated with the new option:

• documentation/docs/30-advanced/50-server-only-modules.md
• documentation/docs/50-api-reference/10-configuration.md

I'd also suggest a different name for the option

But while I was writing this up, Rich came up with possibly a better solution. I thought I'd leave my notes here in case we end up coming back to this solution, but for the meantime it's probably not worth addressing these items

[benmccann]

I believe the rollup check is working and it's only the vite check that's broken, so I would remove this and have it only affect dev and not build

[brillout]

Also, I'm thinking an alternative would be some kind of internal list of exceptions. It's hackish but on the other hand it would be a practical solution that doesn't involve exposing a new config.