kaisermann / svelte-loadable

Dynamically load a svelte component
MIT License
320 stars 13 forks source link

Load sooner? #12

Closed CaptainN closed 5 years ago

CaptainN commented 5 years ago

I noticed the load method invocation is delayed until after the tree renders at least once (it kicks off in onMount). If it were to load immediately, and if the promise would resolve immediately (it should if the import has already resolved), it would render the loadable component immediately, instead of always waiting at least a tick.

This would be useful for cases like rendering on the server for SSR (after preloading the necessary loadables) and could speed up hydration on the client side.

Before submitting a PR, I wanted to check and see if there is something I'm missing as to why this is being kicked off in onMount instead of just starting immediately (I'm super green with Svelte - loving what I see, but still very new with it).

CaptainN commented 5 years ago

moving the load to initialization does make it a bit faster, but I wonder whether there is another gain to be had by somehow skipping the loader in cases when it's already been loaded. Right now I'm guessing that it always takes at least 1 tick before the promise resolves, even if it's previously been resolved. Maybe we can keep a registry based on Map for the first time a loader is loaded. If it's been loaded once already (for example, if a user renders on one route, goes to another, then comes back), we can render immediately instead of waiting.

Some psuedo code:

<script context="module">
const loadedComponents = new Map()
</script>
<script>
// in the place where we currently figure out the module stuff:
    try {
      const componentModule = await loader()
      state = STATES.SUCCESS
      component = componentModule.default || componentModule
      loadedComponents.set(loader, component)
    } catch (e) {
      state = STATES.ERROR
      error = e
    }

// down below where we call "load()"
if (loadedComponents.has(loader))
  state = STATES.SUCCESS
  component = loadedComponents.get(loader)
} else {
  load()
}
</script>

I'll make a PR for this

CaptainN commented 5 years ago

One problem I noticed with the cache solution I proposed, is it will only work for top level components in a long living component, like those loadables defined in the root of an application (App.svelte). If the lifecycle reruns the initialization block, it'll create new loader references, and that'll invalidate the items in the cache. That's a tricky problem to solve within the constraints of the current API. I don't know if there is enough value in this version of a cache in this case, to include it in the core package.

In my wrapper tool for SSR, I'll solve that by requiring loader functions be defined in module code, so that they are only ever defined once (and I also have an additional check to make sure if 2 loaders reference the same module, they'll share the same 1 loader). The API ends up looking more like how react-router works, where loadables are created at definition time, rather than at runtime in the render tree.

It might be worth it to do a minor or major version bump, and modify things in this package to achieve this level of efficiency. Once the loaders have loaded a component, it should render those immediately, but the way it is now, they will always have to wait a couple of ticks.

kaisermann commented 5 years ago

@CaptainN Thanks for this great in-depth study about SSR on both #12 and #13!

You are completely right about moving the loading so it happens before the onMount. It seems there was no actual reason for it (except for my own mistake hahahah). I'm going to release a version with only that change for now.

Now, talking about those other SSR changes you proposed, I need some time to read and study it all 😁. I have little to no experience with SSR, but I'm interested in your proposal on #13, and, as you said, it would be really great to maintain the current API and somehow extend it to improve the SSR experience.

kaisermann commented 5 years ago

Just released v1.2.0 with the loading happening before the onMount cycle.