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

lifecycle-methods (onMount/onDestroy) are failing when called inside an installed package in dev #2147

Closed
benbender opened this issue Aug 9, 2021 · 19 comments
Labels

Comments

@benbender
Copy link

benbender commented Aug 9, 2021

Describe the bug

If I try to use onMount()/ onDestroy() inside an installed 3rd-party-module, it errors in dev-mode but works in prod/preview with Error: Function called outside component initialization.

The exact same code works flawlessly if copied into a local module (see repro @ https://github.com/benbender/sveltekit-lifecycle-repro-ts/blob/master/src/routes/index.svelte#L2).

Noticed the problem at first while tinkering with a port of react-query/core to svelte, noticed it again today while looking into sswr. So it's not a problem of a particular package.

Update:
In next.146 the bug can be fixed by adding the problematic package to vite.optimizeDeps.exclude. Example in https://github.com/benbender/sveltekit-lifecycle-repro-ts/tree/workaround

Reproduction

  1. clone + install https://github.com/benbender/sveltekit-lifecycle-repro-ts
  2. run in dev and switch between the installed and the local copy of the code
  3. see it fail

Logs

proxy.js:15 [HMR][Svelte] Unrecoverable error in <Index>: next update will trigger a full reload
logError @ proxy.js:15
Proxy<Index> @ proxy.js:370
create_if_block_2 @ root.svelte? [sm]:38
create_default_slot @ root.svelte? [sm]:37
create_slot @ index.mjs?v=6a55c099:69
create_fragment @ layout.svelte:21
init @ index.mjs?v=6a55c099:1799
Layout @ layout.svelte:97
createProxiedComponent @ svelte-hooks.js:245
ProxyComponent @ proxy.js:239
Proxy<Layout> @ proxy.js:339
create_fragment @ root.svelte? [sm]:36
init @ index.mjs?v=6a55c099:1799
Root @ root.svelte? [sm]:16
createProxiedComponent @ svelte-hooks.js:245
ProxyComponent @ proxy.js:239
Proxy<Root> @ proxy.js:339
_init @ start.js:681
start @ start.js:563
async function (async)
start @ start.js:515
start @ start.js:1141
(anonymous) @ (index):15
index.mjs:931 Uncaught (in promise) Error: Function called outside component initialization
    at get_current_component (index.mjs:931)
    at onMount (index.mjs:938)
    at l.useSvelte (index.js:1)
    at useSWR (index.js:1)
    at instance (index.svelte? [sm]:4)
    at init (index.mjs?v=6a55c099:1784)
    at new Routes (index.svelte? [sm]:9)
    at createProxiedComponent (svelte-hooks.js:245)
    at new ProxyComponent (proxy.js:239)
    at new Proxy<Index> (proxy.js:339)

System Info

System:
    OS: macOS 11.4
    CPU: (8) arm64 Apple M1
    Memory: 148.09 MB / 8.00 GB
    Shell: 3.3.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 16.6.0 - /opt/homebrew/bin/node
    Yarn: 1.22.10 - /opt/homebrew/bin/yarn
    npm: 7.19.1 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 92.0.4515.131
    Firefox: 90.0.2
    Safari: 14.1.1
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.146
    svelte: ^3.34.0 => 3.42.1

Severity

serious, as I think at least any slightly advanced data-fetching-library for svelte will need to hook into the component lifecycle. At the moment those libraries are practically unusable for endusers as they won't be able to develop with them. There are for sure more usecases where this will be a problem, but this is mine ;)

Additional Information

@benbender benbender changed the title lifecycle-methods (onMount()/onDestroy) are failing when called inside an installed package in dev lifecycle-methods (onMount/onDestroy) are failing when called inside an installed package in dev Aug 9, 2021
@ConsoleTVs
Copy link

Seeing this as a blocker in the 'sswr' pkg

@dominikg
Copy link
Member

#2137 could have fixed this. please update sveltekit to next.146 and try again

@benbender
Copy link
Author

benbender commented Aug 10, 2021

@dominikg Sadly same error occurs in v146 :/

@dominikg
Copy link
Member

sswr does not set the "svelte" field in its package.json https://github.com/ConsoleTVs/sswr/blob/master/package.json which is probably fine as it doesn't export svelte components.
But because of that it isn't detected by the fix i linked above. For now you can add it to kit.vite.optimizeDeps.exclude and kit.vite.ssr.noExternal in your svelte.config.js

@benbender
Copy link
Author

benbender commented Aug 10, 2021

@dominikg Oh, yeah! Just kit.vite.optimizeDeps.exclude fixes it as a workaround.

vite: {
      optimizeDeps: {
        exclude: ["sswr"],
      },
    },

@ConsoleTVs could you add the svelte-field to package.json for testing?

If that's enough, it should be in the documentation and we are done here :)

Edit: Added a "workaround"-branch to the repro: https://github.com/benbender/sveltekit-lifecycle-repro-ts/tree/workaround

@ConsoleTVs
Copy link

@benbender @dominikg Why is there the need for me to add a non standard key into the package.json? Would an empry object be enough or what am I expected to put into thst svelte field?

@benbender
Copy link
Author

@ConsoleTVs If I got this correctly, you'll have to publish the src with the package and have the svelte-field point to it. So it would be "svelte": "src/index.js",

Basically the svelte-field is used by rollup-plugin-svelte and (if properly configured) svelte-loader to locate the source file, so that your third party components get compiled at the same time as the rest of your app (and import from the same internal library).

See https://github.com/sveltejs/rollup-plugin-svelte#pkgsvelte

@dominikg
Copy link
Member

I'm not exactly sure if setting the "svelte" field is the right thing to do for a library that does not export svelte components.
In this case the closest existing file that would make sense is the esm dist, so "svelte":"dist/esm/index.js"
Otherwise you'd have to add an empty index-svelte.js somewhere in dist an ref that. Please do not point to a file that does not exist as that would likely break when bundler plugins try to resolve via the svelte field.

@benbender
Copy link
Author

@dominikg I'm thinking you are on the right track, but as rollup-plugin-svelte states in its docs: "...then this plugin will ensure that your app imports the uncompiled component source code.".

Based on this statement and on my repro, I think it is needed to ship the source inside the package and it will, basically, automate my initial workaround of "copying" the source of the package into the actual app in dev-mode. This works as intended as proven by my repro and I think it would be the solution to this (even if it isn't, obviously, the most elegant solution in the world...)

@dominikg
Copy link
Member

thats the thing, there are no components in sswr. It is a utility library with a peer dependency on svelte.
Unfortunately vite optimizer currently does not deduplicate svelte correctly, so for it to work you have to exclude libraries from optimization until vite is fixed. Thats being worked on right now.
Your copy to src solution basically bypassed the optimizer and therefore worked. sswr doesn't need to distribute the source of its utility.

In case you want to dive into the rabbit hole
sveltejs/vite-plugin-svelte#125
vitejs/vite#4230 (comment)

@ConsoleTVs
Copy link

So not only I have to add that key but also the source code to the npm files????

I am starting to feel a bit bissed at this point...

@dominikg
Copy link
Member

@ConsoleTVs no you don't, see my comment above. You could add it to enable the automatic workaround, or document somewhere that it needs to be added to optimizeDeps.exclude manually.

At some point in the future this problem will be fixed, either in vite itself or in vite-plugin-svelte

@benbender
Copy link
Author

@dominikg Thanks for the explanation. You are probably right 👍

@ConsoleTVs
Copy link

@dominikg @benbender Thanks for the hard work here, I will add this note to the docs and wait for official fixes! Great work to both of you!

@frederikhors
Copy link
Contributor

I think this fix it (for now): ConsoleTVs/sswr#11.

@benbender
Copy link
Author

benbender commented Aug 10, 2021

@frederikhors not really as this issue isn't about sswr and the underlying problem isn't fixed.

It could be closed instead as a bug in vite or rollup-plugin-svelte. As I'm not sure how to handle this further and if this needs to be tracked here, I would like to delegate the decision to @dominikg, who is much deeper into the topic :)

@benmccann benmccann added the vite label Aug 10, 2021
@bluwy
Copy link
Member

bluwy commented Aug 11, 2021

Note: Once sveltejs/vite-plugin-svelte#137 is released for vite-plugin-svelte and upgraded in SvelteKit, the workaround re optimizeDeps.exclude is not needed anymore.

@dominikg
Copy link
Member

vite-plugin-svelte 1.0.0-next.16 has been released: https://github.com/sveltejs/vite-plugin-svelte/blob/main/packages/vite-plugin-svelte/CHANGELOG.md

@benbender
Copy link
Author

@dominikg I'm happy to report that this issue is fixed with the release. Awesome work! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants