Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Promise and lazy routes... #104

Closed
frederikhors opened this issue Apr 10, 2020 · 11 comments
Closed

Promise and lazy routes... #104

frederikhors opened this issue Apr 10, 2020 · 11 comments
Labels
enhancement New feature or request
Milestone

Comments

@frederikhors
Copy link
Contributor

Looking at yrv router (a good router!) we can see two ways of importing routes:

<Router>
  <Route exact path="/promise" component={import('path/to/other-component.svelte')}/>
  <Route exact path="/lazy" component={() => import('path/to/another-component.svelte')}/>
</Router>

With https://github.com/ItalyPaleAle/svelte-spa-router/blob/master/Advanced%20Usage.md#async-route-loading we have just the lazy one. Am I correct?

I think it would be useful (and amazing) to have both in svelte-spa-router (three-shaked if we do not use them).

The promise way I think is amazing because I can have a lot of routes and maybe I just need to load immediately just the one I need and after that in background start downloading others.

What do you think about?

@ItalyPaleAle
Copy link
Owner

This is a valid point and related to #73.

Given the work @hmmhmmhm has done, which I think is really good, I haven't put much effort trying a first-party solution for #73.

However, I'm open to accept PRs :)

@frederikhors
Copy link
Contributor Author

frederikhors commented Apr 10, 2020

However, I'm open to accept PRs :)

I wish I was able to do!

@ItalyPaleAle I found many issues with svelte-spa-chunk (also with style code in it: https://github.com/hmmhmmhm/svelte-spa-chunk/blob/master/Chunk.svelte#L40-L46 which interferes with my application global style).

I'll open issues as soon it works on REPL (hmmhmmhm/svelte-spa-chunk#3).

From some tests that I did it showed an increase of about 10 KB using it (but testing it only with 3 routes).

Not bad but I think svelte-spa-router needs something internal, three-shackable, both promising and lazying, with pending prop (or something else to indicate a pending component to show during wait, like yrv already does very well).

And also the code should be on this repository, with the tests and releases aligned.

Can I ask you why you "shy away" the idea of using your magnificent mental abilities to equip us, mortals, with an essential feature like this?

@frederikhors
Copy link
Contributor Author

frederikhors commented Apr 11, 2020

See what yrv does for this: https://github.com/pateketrueke/yrv/blob/master/src/Route.svelte#L75-L91:

  $: if (activeRouter) {
    if (!component) { // component passed as slot
      hasLoaded = true;
    } else if (isSvelteComponent(component)) { // component passed as Svelte component
      hasLoaded = true;
    } else if (isPromise(component)) { // component passed as import()
      component.then(module => {
        component = module.default;
        hasLoaded = true;
      });
    } else { // component passed as () => import()
      component().then(module => {
        component = module.default;
        hasLoaded = true;
      });
    }
  }

@frederikhors
Copy link
Contributor Author

frederikhors commented Apr 11, 2020

I tried some code and it works using it like this:

export const routes = new Map().set(/^\/players(\/(.*))?/i, () => import('./pages/players/Page.svelte'))

It loads route but I'm having issues with this:

Router.svelte:182 Uncaught (in promise) TypeError: Class constructor Page cannot be invoked without 'new'
    at Router.svelte:182

@frederikhors
Copy link
Contributor Author

frederikhors commented Apr 11, 2020

I think we do not need svelte-loadable (which svelte-spa-chunk uses).

Code splitting comes free from Rollup.

We need only to handle promise and lazy import in Router.svelte.

I think you can do it in a few minutes, @ItalyPaleAle.

@hmmhmmhm
Copy link
Contributor

@frederikhors

First of all, I will answer the issue that occurred in my chunk module in the post you left.
( hmmhmmhm/svelte-spa-chunk#3 )

I am currently providing an example of Code Splitting in the form of a template based on Parcel.
( https://github.com/hmmhmmhm/svelte-spa-template )

The last time I tried the split code in Rollup, there were a lot of problems.
When you called the bit code through import, did you check if the code was really split?

The last time I checked in Rollup, it looked like it was well separated,
but it didn't physically generate two files.

Once you download all the files, you can see that the code processing for the purpose of deciding whether or not to upload them to memory is already done by the svelte-spa-router.

The code that simply uses import code in brackets is in a way not code splitting.

I used svelte-loadable to implement code spliting + dynamic import
that is compatible with the bundled system of Rollup, Webpack, and Parcel.

If Rollup is a simple method of physical code spliting and that method is available for page-level rendering of svelte-spa-router, please share an example or PR separately.

@frederikhors
Copy link
Contributor Author

@hmmhmmhm thanks for your commitment.

I'm not an expert here. I think your work is valuable. But can you please look at #104 (comment) which comes from yrv router (https://github.com/pateketrueke/yrv)?

yrv is not using svelte-loadable and it works great!

@hmmhmmhm
Copy link
Contributor

@frederikhors OK! I will check it within this week and if possible, I will apply it to the spa-router. I'm sorry that I can't check early because I have a bad cold.

@ItalyPaleAle
Copy link
Owner

Thanks for the great discussion. I think y'all are right and it's time we implement this natively in the router.

@hmmhmmhm let me know if you are able to work on a PR. If you can, I'd greatly appreciate it, but no pressure :)

I'm scheduling this for 3.0

@ItalyPaleAle ItalyPaleAle added this to the v3.0 milestone Apr 18, 2020
@ItalyPaleAle ItalyPaleAle added the enhancement New feature or request label Apr 18, 2020
@frederikhors
Copy link
Contributor Author

Dear @ItalyPaleAle,

I figured out how to use lazy and "preloaded" routes in svelte-spa-router as it is today.

This is an example: https://codesandbox.io/s/svelte-spa-router-lazy-and-promises-routes-7zthn.

Do you have suggestions?

For example I'm already repeating the Root.svelte component for each route:

<script>
  import Lazy from '../../Lazy.svelte'
</script>

<Lazy component={import('./Index.svelte')} />

and for "preloaded" routes I'm using this:

import("./routes/preloaded/Index.svelte")

which maybe is not very elegant.

What do you think?

Thanks again for your work! This is the best router today and I say it because I tried them all!

@ItalyPaleAle
Copy link
Owner

There's currently work being done in #73 to support dynamically-imported routes.

Once that's implemented, it will be possible to do a lazy loading too, by invoking import('foo.svelte') in your init code in main.js. Subsequent invocations to import, including in async routes as supported by #73, will just return the cached code.

I'll write some docs on how to do this

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants