kit icon indicating copy to clipboard operation
kit copied to clipboard

Redirects within packages interpreted as errors when using pnpm link

Open sbmw opened this issue 1 year ago • 1 comments

Describe the bug

This is maybe a bit niche...

When building a svelte package and an app in unison, it can be very useful to use pnpm link to override the app's package dependency with the package's local directory. Working in this way provides HMR benefits without needing to make any hard edits to the app's package.json dependencies.

A redirect (eg redirect(307, "https://google.com");) within the package will be handled as an error when working in this manner.

The underlying cause seems to be the same as a few previous issues (eg https://github.com/sveltejs/kit/issues/9234) but the previous noExternal solutions don't work, for me at least.

Reproduction

Very basic reproduction linked below. The 4 edited files and steps to reproduce are listed in the readme.

https://github.com/sbmw/sveltekit-pnpmlink

In short:

In the package - pnpm i && pnpm build In the app - pnpm i && pnpm link ../my-package && pnpm dev

https://localhost:5000/foo - error thrown from server load https://localhost:5000/login - error thrown from server hook

Logs

PATH /foo
Redirect { status: 307, location: 'https://google.com' }
PATH /login
Error: {"status":307,"location":"https://github.com/login"}
    at Module.coalesce_to_error (/home/foo/sveltekit-pnpmlink/my-app/node_modules/.pnpm/@[email protected]_@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/utils/error.js:11:5)
    at Module.handle_fatal_error (/home/foo/sveltekit-pnpmlink/my-app/node_modules/.pnpm/@[email protected]_@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/runtime/server/utils.js:72:47)
    at Module.respond (/home/foo/sveltekit-pnpmlink/my-app/node_modules/.pnpm/@[email protected]_@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/runtime/server/respond.js:394:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async file:///home/foo/sveltekit-pnpmlink/my-app/node_modules/.pnpm/@[email protected]_@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/exports/vite/dev/index.js:524:22

System Info

System:
    OS: Linux 6.6 Fedora Linux 38 (KDE Plasma)
    CPU: (16) x64 12th Gen Intel(R) Core(TM) i7-1260P
    Memory: 43.93 GB / 62.51 GB
    Container: Yes
    Shell: 5.2.21 - /bin/bash
  Binaries:
    Node: 21.6.1 - ~/.n/bin/node
    npm: 10.2.4 - ~/.n/bin/npm
    pnpm: 8.14.3 - ~/.n/bin/pnpm
  npmPackages:
    @sveltejs/adapter-auto: ^3.0.0 => 3.1.1 
    @sveltejs/kit: ^2.0.0 => 2.5.0 
    @sveltejs/vite-plugin-svelte: ^3.0.0 => 3.0.2 
    svelte: ^4.2.7 => 4.2.9 
    vite: ^5.0.3 => 5.0.12

Severity

annoyance

Additional Information

One workaround is to use the file: protocol, but this loses the ease/hmr benefits and you need to remember to revert the changes before pushing app code.

"dependencies": {
    "my-package": "file:../my-package"
},

Or am I just using the wrong approach for working on a package?

sbmw avatar Jan 27 '24 18:01 sbmw

After a few partial work-around solutions, the winner for me is yalc instead of pnpm link

# Setup
pnpm i -g yalc
## In the svelte package project
yalc publish
## In the svelte app project
yalc link <package> (and some .gitignore updates)

# Dev
pnpm build && yalc publish --push

Works great!

sbmw avatar Feb 14 '24 15:02 sbmw

@eltigerchino but... it's not same kind of problem with my issue #11901, or i missed something, no ?

piolet avatar Feb 26 '24 15:02 piolet

@eltigerchino but... it's not same kind of problem with my issue #11901, or i missed something, no ?

My suspicion is that Kit is unable to correctly identify the thrown error when it is thrown from a different package other than the current project.

Internally, Kit does a if (error instanceof HttpError) check where HttpError (or Redirect) is the thrown class instance when you use a error or redirect helper. However, this is incorrectly evaluating to false (because the external package has its own installation of Kit, therefore throws a different kind of HttpError?)

eltigerchino avatar Feb 26 '24 16:02 eltigerchino

but then why was it able to handle it properly in version 1.30? What's the difference between version 2.50 and 1.30, apart from the disappearance of the "throw" (at least according to the migration page ;) )?

piolet avatar Feb 26 '24 16:02 piolet

@eltigerchino so, to be able to use kit 2 with a monorepo and good error handling, are you suggesting that I link my sub-repo to my main repo, as mentioned in this issue?

sorry to ask, it's just to understand and try to move forward on my side ;)

piolet avatar Feb 27 '24 10:02 piolet

@eltigerchino so, to be able to use kit 2 with a monorepo and good error handling, are you suggesting that I link my sub-repo to my main repo, as mentioned in this issue?

sorry to ask, it's just to understand and try to move forward on my side ;)

You can try to ensure that your packages are using the same vite, @sveltejs/kit, svelte, and @sveltejs/vite-plugin-svelte versions so that the HttpError class is imported from the same module. Otherwise, they end up importing from different node_module installations and fail on the instanceof check. This is because pnpm has different installations for each combination of package versions.

list of different pnpm node_module installations of sveltekit

eltigerchino avatar Feb 27 '24 13:02 eltigerchino

Thx @eltigerchino . In my issue (#11901 ) i joined a basic repo who explained the behavior and how to reproduce it simply (https://github.com/piolet/error-repo).

You can see 2 repo with same dependencies's version. Without cache, from scratch, i got my wrong behavior

(maybe we could reopen my issue to discuss about that, no ? )

piolet avatar Feb 27 '24 13:02 piolet

Thx @eltigerchino . In my issue (#11901 ) i joined a basic repo who explained the behavior and how to reproduce it simply (piolet/error-repo).

You can see 2 repo with same dependencies's version. Without cache, from scratch, i got my wrong behavior

(maybe we could reopen my issue to discuss about that, no ? )

In the service and frontend packages, you have installed Vite 4. However @sveltejs/vite-plugin-svelte in the frontend package requires Vite 5, so pnpm installs Vite 5 in the frontend package but not the service package. This creates two different node_module installations by pnpm.

CleanShot 2024-02-27 at 10  06 33@2x

Please note that SvelteKit 2 and vite-plugin-svelte 3 require Vite 5.

eltigerchino avatar Feb 27 '24 14:02 eltigerchino

So thank you very much... you saved my ... week ;)

piolet avatar Feb 27 '24 14:02 piolet

This is a valid error with npm link too, with identical package versions (just updated both packages as this was my suspicion too).

To be clear this is with:

dependencies: {
  "package": "file:../../path"
}

It's hard to provide an exact repo, but the workaround is to just return a Response from there, or catch the errors yourself.

rudiv avatar Apr 11 '24 17:04 rudiv