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

docs: expand & clarify FAQ entry on using client-only libraries #10886

Merged

Conversation

HoldYourWaffle
Copy link
Contributor

While helping some beginners with their first Svelte(Kit) project I encountered some major confusion around using client-only libraries with SSR. I have tried to improve this section of the documentation based on that feedback.

Some notes:

  1. English do be difficult, feel free to correct my mediocre writing skills :)
  2. The warning added in 880d610 would've saved us a lot of time debugging, but perhaps it's a better fit for the section on onMount?
  3. The example added in 5c62f9b suffers from {#await} with a null-ish expression renders fullfiled branch during SSR svelte#9323, so I feel like a comment explaining what's going on is appropriate. I'm not sold on this particular format though, any suggestions?
  4. Git's diff came out a little weird making this very uncomfortable to review... Sorry!

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. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

@changeset-bot
Copy link

changeset-bot bot commented Oct 17, 2023

⚠️ No Changeset found

Latest commit: 6cf7255

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

In particular mention dynamic imports more explicitly.
Note that using `Promise.resolve()` in the server branch would make TS angry, since it's of type `Promise<void>`.
Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the FAQ way too long and complicated. Also, I think that the new examples are solving the problem at the wrong level. We should be making Svelte components that work only on the client-side as are demonstrated being invoked here

@HoldYourWaffle
Copy link
Contributor Author

@benmccann Conceptually I agree, this feels too "significant" to be tucked away in an FAQ section.
Perhaps the section on "How to use X" or maybe just "How to use client-libraries" deserves its own page?

I went through several iterations, but so far I haven't found a way to condense this further without being confusing/overwhelming for newcomers.
The current version implicitly assumes knowledge about dynamic vs static imports and the impact of side-effects, but this isn't common knowledge when you're new to the wondrous world of SSR. It also leads users to dynamically import things in onMount, which can cause major headaches.
I'd love to hear your suggestions though!

I'm not entirely sure what you mean by your second note. I only added variations of existing examples and one for using a client-only component. If I understand correctly you like the latter, so would you like the existing (more basic) examples to be removed then?
I'm not entirely against this, given that it should be covered by other sections of the documentation, though I do see value in summarizing them like this for beginners.

@benmccann
Copy link
Member

I'm saying there should be no such thing as a client-only component. You've already gone down the wrong path if you fine yourself in such a situation. Rather than demonstrating how to use a broken component, we should instruct people to fix the component. That component should follow this FAQ to use the client-only code only in the browser rather than remaining difficult to import.

@HoldYourWaffle
Copy link
Contributor Author

Aaaah that makes a lot more sense. What an impact a small typo can make :P
Again, conceptually I fully agree, client-only components are definitely something to be avoided.

However, some components make such heavy use of client-only code that server-rendering them serves no value. The async nature of dynamic imports and various restrictions around things at the top level make it vastly more complicated to implement them in a way that "supports" SSR. These components don't make sense to render on the server, an empty <div> might be the best you can do, so I would like to avoid jumping through hoops to "pseudo-support" it.

In my case, we were working on a Svelte integration for monaco-editor. Initially we used a basic dynamic import in onMount, but this significantly complicated the implementation and caused mind-numbing race conditions depending on how long it took the browser to import the module.
There's no clear "component is done initializing/rendering" anymore, causing a bunch of problems around half-initialized components.

Making the component client-only and usage explicitly async ended up with much more straightforward code than all the other workarounds I could think of. For bonus points: it's trivial to add a friendly loading spinner with {#await} :)

As much as I hate it, I think client-only components are the better/more natural solution in some cases. I fully agree that it's smelly and tedious though, I'd love to hear your suggestions!

(For reference: an example project on how to use monaco-editor in SvelteKit solves this by disabling SSR for the whole page, I'd argue that's even worse)

@shynome
Copy link

shynome commented Nov 13, 2023

Why delete vite-plugin-iso-import faq? It save me so much time, if I don't find it at faq, I will still be pain in client library support

Today I use import bootstrap?client to implement action, How do you write the blow code by use browser check?

<script context='module'>
import boostrap from 'bootstrap?client'
function tooltip(node){
const inst = new bootstrap.Tooltip(node)
}
</script>
<span title='hello' use:tooltip>tooltip</span>

@HoldYourWaffle
Copy link
Contributor Author

I was wondering the same thing, I feel like @dummdidumm's 6cf7255 condensed a bit too much.

What kind of error do you get if you try your example without ?client?
As far as I know actions only run on the client, so bootstrap.Tooltip shouldn't be called during SSR.

@benmccann
Copy link
Member

benmccann commented Nov 13, 2023

Why delete vite-plugin-iso-import faq?

It felt a bit non-standard and so I thought it was better to focus on the things that are built-in to Svelte and Vite. It is a very cool plugin though. Perhaps it could be added to Vite core at some point if people really find it useful. I wouldn't be against adding back a link to it if folks are finding it useful, but I think that having a demonstration of it in the docs is too much. We can just raise awareness of it and people can reference the plugin's docs if they want to use it

How do you write the blow code by use browser check?

<script>
  import { onMount } from 'svelte';

  let bootstrap;
  onMount(async () => {
    bootstrap = await import('bootstrap'); // or it might be (await import('bootstrap')).default. I don't have it setup to test
  });

  function tooltip(node){
    const inst = new bootstrap.Tooltip(node)
  }
</script>
<span title='hello' use:tooltip>tooltip</span>

I feel like @dummdidumm's 6cf7255 condensed a bit too much.

That was my commit

@HoldYourWaffle
Copy link
Contributor Author

Woooops, no idea how I messed up those names... Sorry!

I can't test it right now, but top-level await doesn't work in Svelte components right? You'd have to wrap the await import in an IIFE or something.
Either way, I worry this approach could create a race-condition similar to the one I proposed adding a warning for. tooltip will be called when the span is mounted, but bootstrap might not have finished importing yet.

@benmccann
Copy link
Member

oops, yeah, I should have used onMount. updated my comment above

@HoldYourWaffle
Copy link
Contributor Author

span mounts before the component though right?
Still can't test it, but I'd expect the action to fire before the component's onMount.

Even if the component's onMount fires first you could still run into a race-condition, since the import is async and might complete way after onMount finishes/tooltip is called.

@shynome
Copy link

shynome commented Nov 14, 2023

FAQ help people deal the actual problem easily

My old code with browser check before know iso-import, it's has more status need manual manage, that mean more code.

<script context='module'>
function tooltip(node){
  const p = import('bootstrap').then(()=>{
    const inst = new bootstrap.Tooltip(node)
    return inst
  })
  return {
    destroy(){
      p.then((inst)=>{
        inst.dipose()
      })
    }
  }
}
</script>
<span title='hello' use:tooltip>tooltip</span>

Svelte take care about devoloper happiness, is that still right?
https://github.com/feltcoop/why-svelte
https://svelte.dev/blog/write-less-code

@HoldYourWaffle
Copy link
Contributor Author

This is definitely an impractical section of Svelte. sveltejs/svelte#5501 would make the syntax less cumbersome, but then you easily fall into the race-condition trap.

@shynome What kind of error do you get if you import bootstrap statically without ?client?
As far as I know actions only run on the client, so bootstrap.Tooltip shouldn't be called during SSR.

@shynome
Copy link

shynome commented Nov 21, 2023

This is definitely an impractical section of Svelte. sveltejs/svelte#5501 would make the syntax less cumbersome, but then you easily fall into the race-condition trap.

@shynome What kind of error do you get if you import bootstrap statically without ?client? As far as I know actions only run on the client, so bootstrap.Tooltip shouldn't be called during SSR.

I don't want to talk more about this, you can do this and I also can work easier than others which don't know about iso-import

https://github.com/shynome/sveltekit-without-isoimport

error log

8:47:38 am [vite] Error when evaluating SSR module /src/routes/+page.svelte: failed to import "/src/lib/index.ts"
|- ReferenceError: document is not defined
    at enableDismissTrigger (/home/shynome/work/tmp/kit-without-iso/node_modules/bootstrap/dist/js/bootstrap.js:826:21)
    at /home/shynome/work/tmp/kit-without-iso/node_modules/bootstrap/dist/js/bootstrap.js:908:3
    at /home/shynome/work/tmp/kit-without-iso/node_modules/bootstrap/dist/js/bootstrap.js:7:83
    at Object.<anonymous> (/home/shynome/work/tmp/kit-without-iso/node_modules/bootstrap/dist/js/bootstrap.js:10:3)
    at Module._compile (node:internal/modules/cjs/loader:1256:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:169:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

@HoldYourWaffle
Copy link
Contributor Author

Aaaah, bootstrap has side-effects, that makes sense.

The most obvious solution is a guarded (onMount/if (browser)) dynamic import, but that leads to race-conditions (#10886 (comment)) and/or a bunch of really verbose and tangled then-ing hoop jumping (#10886 (comment)).

Sounds like a perfect usecase for vite-plugin-iso-import's ?client: a static import that is forcibly erased in the SSR build.
I don't see how regular tree-shaking could do the job here. There are (client-only) side-effects and the "replace with no-op" strategy of "only used in onMount" doesn't work with actions :(

@benmccann Any thoughts?
I agree that it's a bit "non-standard", but it does seem to solve an important gap in Svelte's "SSR with client-only code" mechanics.

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

Successfully merging this pull request may close these issues.

3 participants