sveltejs / sapper

The next small thing in web development, powered by Svelte
https://sapper.svelte.dev
MIT License
7k stars 433 forks source link

Calling the preload function needs more fine grained control #1308

Open ian-callaghan opened 4 years ago

ian-callaghan commented 4 years ago

Is your feature request related to a problem? Please describe. Currently preload is called whenever the session/page store are updated client side, from reading my understanding that this is intentional and desired in many pages for various reasons.

However this is behaviour is prohibitive in various scenarios, especially in scenarios where async api calls are performed in the preload function.

An example would be when we are on a product listing page, we initially load 10 products in preload and then progressively load more products:

products.svelte

<script context="module">
    import * as api from "../api"

    // We want this called only once ever, either SSR or client side
    export async function preload(page, session) {
        const response = await api.getData()
        return { response }
    }
</script>

<script>
    export let response

    const onLoadMore = async _ => {
        const nextResponse = await api.getData({page:response.next})
        nextResponse.results = response.results.concat(nextResponse.results)
        response = nextResponse
    }
</script>

In this case we're loading unauthenticated data to display, if a user logs in/out on the page and we update the session store, calling the initial endpoint again becomes redundant.

A second scenario is if we update the page store, for example changing the query params to show additional content that can also be deep linked.

header.svelte

<a href={`${$page.path}?login=1`}>Log In</a>

_layout.svelte

<script>
    import { stores } from "@sapper/app"
    const { page } = stores()
    $: loginOpen = $page.query.login
</script>

{#if loginOpen}
    <LoginModal />
{/if}

Again in this scenario, the update to the query store is of no relevance to products.svelte preload function but it will run regardless. Even worse, because this change is reflected in the view the preload function actually blocks rendering which can make an app appear unresponsive/slow.

Describe the solution you'd like I think an ideal solution is to allow more fine grained control of the preload function. Currently it's very isolated only having direct access to page/session object. I think 2 separate enhancements could improve this for various scenarios:

  1. Use a "reducer style" pattern and pass in the previous session/page to preload e.g. preload(page, session, previous). This would allow the current default behaviour as well as enabling a degree of control through comparisons e.g.

    export async function preload(page, session, previous) {
        if (previous.value.page.path !== page.path) {
            const response = await api.getData(...)
            return { response }
        }
        else {
            return { ...previous.props }
        }
    }
  2. Enable caching of data so that it can be retrieved for pages:

    export async function preload(page, session) {
            let productData = this.cache.get(`${page.query.productId}`)
            if(!productData) {
                productData = await api.getProduct(page.query.productId);
                this.cache.set(`${page.query.productId}`, productData)
            }
            const stockLevel = await api.getStock(page.query.productId)
    
            return { productData, stockLevel }
        }
    }

Describe alternatives you've considered A current workaround for (1) is to use a custom store that is controlled from the root _layout.svelte, but I think this should be included as a common feature to avoid boilerplate:

stores.js

function createCurrent() {
    const defaultValue = {
        page: { path: null },
        session: null,
    }
    const { subscribe, set, update } = writable(defaultValue)

    let value
    subscribe((v) => (value = v))

    const reset = () => set(defaultValue)

    return {
        subscribe,
        update,
        set,
        reset,
        get value() {
            return value
        },
        setPage: (page) => {
            update((cur) => ({
                ...cur,
                page,
            }))
        },
        setSession: (session) => {
            update((cur) => ({
                ...cur,
                session,
            }))
        },
    }
}

export const current = createCurrent()

_layout.svelte

<script>
    import { onDestroy } from "svelte"
    import { stores } from "@sapper/app"
    import { current } from "../components/stores"
    const { session, page } = stores()

    page.subscribe(v => current.setPage(v))
    session.subscribe(v => current.setSession(v))

    onDestroy(async () => current.reset())
</script>

products.svelte

<script context="module">
    import { current } from "../components/stores"

    export async function preload(page, session) {
        if (current.value.page.path !== page.path) {
            const response = await api.getData(...)

            return { response }
        }
    }
</script>

For (2) using various api's like localStorage or equivalent SSR can be used but a clean universal way to do this would be ideal.

How important is this feature to you? Very important, without this feature a lot of common scenarios cannot easily be done with sapper without complex workarounds.

stephane-vanraes commented 4 years ago

I like the first idea of somehow having the previous page available in the preload.

Another use case would be the following 1) user attempts to access the private page /myprofile 2) gets redirected to /login 3) on login is redirected back to /myprofile

This would become trivial if the login preload has access to the page you came from.

A variant of the (page, session, previous) could be to keep (page, session) but have page be { path, params, previous, query } instead

PatrickG commented 4 years ago

You could do your first solution yourself.. kinda

Something like this might work for you:

// preload.js
let previous = {
    page: undefined,
    props: undefined,
};

export const pipe = (...fns) => async function (page, session) {
    let props;

    for (const fn of fns) {
        const result = await fn.call(this, page, session, previous);
        props = props ? { ...props, ...result } : result;
    }

    if (process.browser) {
        previous = { page, props };
    }

    return props;
}
<!-- products.svelte -->
<script context="module">
    import { pipe } from './preload';
    import * as api from './api';

    export const preload = pipe(async (page, session, previous) => {
        if (previous.page.path === page.path && previous.props) {
            return { ...previous.props };
        }

        return { response: await api.getData() };
    });
</script>

...
I'm using a `pipe`-like function to execute multiple function like: _maybe pipe is not a good name for this^^_ ```html ... ```
antony commented 4 years ago

I've actually recently implemented server and client-side pagination in a pretty clean way and not had any issues with the way preload is executed. I'll try to write a recipe / how-to on how I've done this as soon as I get the opportunity.

ian-callaghan commented 4 years ago

I've actually recently implemented server and client-side pagination in a pretty clean way and not had any issues with the way preload is executed. I'll try to write a recipe / how-to on how I've done this as soon as I get the opportunity.

That would be very useful, it would be good to know how to do this without complicated workarounds.