sveltejs / sapper

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

Feature request: Preload argument for context #751

Closed hitsthings closed 3 years ago

hitsthings commented 5 years ago

Problem

I've been trying to adopt Sapper the past couple weeks, and it's been mostly good. One topic seems to keep biting me though, and that has to do with "initializing" each request in various ways so that I can use my helpers in the client or on the server. For example, here are a couple things I wanted to do:

Workaround

I have so far been abusing _layout.svelte's preload to ensure something happens on the server, and all of my helpers have a process.browser check to change behavior (and change which things are require()'d) in browser and server. I have abused session to share a property between all the preloads and all the ssr renders - in _layout.svelte's preload I do session.apolloClient = .... and in _layout.svelte's render I do setContext(key, $session.apolloClient); $session.apolloClient = null;. This is pretty ugly, but it's the best I've got at the moment.

Solution?

I'd like to help implement something in Sapper to deal with this, if you have an guidance on what you want to see or whether this interests you.

My thinking was that a third parameter could be passed to preload which essentially acts like Svelte context - it doesn't persist between client and server, but is available during preload. E.g.

<script context="module">
    export async function preload(page, session, context) {
        // set up some context both for preloads and for components
        // not the best place to do this though, see below for where I would actually do it.
        context.set('apolloClient', createClient(this.fetch)); 

        // a preload lower down the tree could do
        return {
            data: await context.get('apolloClient').query(....)
        };
    }
}
</script>
<script>
    getContext('apolloClient') // would only work during the server-side render, unless `preload` was called client-side
</script>

This third parameter could be initialized similarly to session with server/browser-specific details, which avoids lots of process.browser switches throughout my code. Something like: (client.js)

sapper.start({
    target: ...,
    context: () => [
        ['apolloClient', createClient()],
        ['mixpanel', window.mixpanel],
        ['honeycomb', honeycombProxy]
    ]
})

(server.js)

sapper.middleware({
    session: (req, res) => { ... }
    context: (req, res) => [
        // use cookies from the request when talking to API server
        ['apolloClient', createClient(req) ]
        // associate any events with the user on this request
        ['mixpanel', {
            track: (event, props) => require('mixpanel').track(event, {
                distinct_id: req.user.id,
                ...props
            })
        ],
        // honeycomb does magic to hook into express at module load, so no explicit user link is necessary
        ['honeycomb', require('honeycomb-beeline')() ]
    ]
})

~Note I used an array-of-arrays to initialize because Context keys can be both objects and strings (so neither a Map nor an object literal are sufficient).~ EDIT: Just realized I was thinking of WeakMaps which can't accept string. A Map is what's actually used, so returning a Map from this function would be just fine! Perhaps some other API is better.

Something like the above would solve my problem of overusing process.browser, and would let me pull helper logic out of _layout.svelte that doesn't really belong there. Most importantly I could stop abusing session.

I'm keen to hear what you think and again - I'm happy to put in work for this if you think it has legs.

Cheers!

hitsthings commented 5 years ago

Haven't heard anything, but it feels like it wouldn't be terribly difficult to implement so I think I will give it a try. Still, any guidance you can give would be great. Telling me "no" would also be appreciated before I put in the effort. :)

One change I'd potentially make is to actually find a way to run the context callback within App.svelte tree and allow direct getContext/setContext calls that people are used to. E.g.

import { setContext } from 'svelte';
sapper.start({
    context: () => {
        setContext('apolloClient', createClient());
        setContext('mixpanel', window.mixpanel);
        setContext('honeycomb', honeycombProxy);
    }
})

Is that a better API? Should it still be called context then, or init since it's more generic? Doing it that way has more unanswered questions since I'd have to move preload calls into App.svelte as well and then somehow wait for them to complete before beginning the SSR of components inside App.

utherpally commented 5 years ago

My solution, work with both side.

//apollo.js
const clients = {}
export const getClient = (id) => {
    if(id in clients) {
    eturn clients[id]
    }
    return (clients[id] = createApolloClient())
};
export const removeClient = (id) => delete clients[id];

// server.js
import {removeClient} from './apollo';
const app = sapper.middleware({
    session: (req, res) => {
        const apolloID = uuidv4();
        res.once('finish', function() {
        removeClient(apolloID)
    )
    return { apolloID }
    }
})
<script context="module">
    import { getClient } from '../apollo'
    export async function preload(_, session) {
    const client = getClient(session.apolloID)
   }
</script>

But how about caching? Sapper don't have something like 'page template', so I can't inject apollo script below in sever-side.

<script>
  window.__APOLLO_STATE__=${JSON.stringify(cache.extract()).replace(/</g, '\\u003c')}
</script>

Maybe add new feature request ?

sapper.middleware({
    session: () => {},
    pageRender: (body /* template after replace sapper placeholder code */, context) => {

    }
})
ajbouh commented 5 years ago

Can't we just put the output of cache.extract() into the session object? Seems like sapper will handle the JSON.stringify and JSON.parse for us in that case.

justinmoon commented 4 years ago

This has been my biggest issue with Sapper so far

simoncpu commented 4 years ago

Greetings from the future! I'm a time traveler from the year 2020. This is also the issue that I'm currently experiencing, specifically with Apollo Client.

ajbouh commented 4 years ago

I've devoted a decent quantity of effort to working around this feature's absence. FWIW, using the clientId approach above seems to be the cleanest workaround so far.

simoncpu commented 4 years ago

I've just tried the client ID approach and it works well. Thanks!

benmccann commented 4 years ago

@langbamit can you share what your createApolloClient looks like?

I'm wondering if you use Sapper's this.fetch when creating the client to support isomorphic rendering. Or do you provide your own fetch polyfill using something like isomorphic-fetch or cross-fetch. If the latter, when making a server-side request will you still have the request headers from the original request carried over to pass cookies and auth headers?

bluwy commented 3 years ago

On the topic of initializing per-request Apollo Clients, I found the below code to work best for me. No global map needed, directly set the apollo client in the session object. The trick here is to mutate the session during component rendering to make the session serializable, and then hydrate the client again in the client-side. This relies on the fact that Sapper serializes the session only after the app SSR-ed.

server.js:

// ...
sapper.middleware({
  session: () => ({
    // Instantiate client, but can't serialze? No problem, we'll fix this later
    apollo: new ApolloClient({
      // Make sure queries run once
      ssrMode: true,
      // Use SchemaLink to run queries on local schema (no round trips)
      link: new SchemaLink(...),
      // Collects cache for this session
      cache: new InMemoryCache(),
    })
  })
})
// ...

_layout.svelte:

<script>
  import { stores } from '@sapper/app'
  import { onDestroy } from 'svelte'
  import { apollo } from '../apollo' // imports client-side Apollo Client, undefined in server

  const { session } = stores()

  if (!process.browser) {
    onDestroy(() => {
      // Replace apollo client with its cache for serialization
      $session.apollo = $session.apollo.extract()
    })
  } else {
    // Restore the cache string back
    apollo.restore($session.apollo)
    // At client-side, the `$session.apollo` should refer to the client-side version
    $session.apollo = apollo
  }
</script>

For clarity, apollo.js:

export const apollo = process.browser
  ? new ApolloClient({
      uri: '/graphql',
      cache: new InMemoryCache(),
      ssrForceFetchDelay: 100,
    })
  : undefined

At this point, you could even return req and res for the session function, and "fix" it later, provided that you only access it in the server. But it's probably better to stay with Sapper's original intent of session.

UPDATE: I've written two best solutions I've found so far in my blog, including the above, if anyone is interested.

ajbouh commented 3 years ago

This is a nice approach!

benmccann commented 3 years ago

Closing as duplicate of https://github.com/sveltejs/kit/issues/327

eikaramba commented 3 years ago

too bad that it is closed, i have given up on sapper and svelte-kit because of this

ajbouh commented 3 years ago

It looks like the issue this has been marked as a duplicate of does not allow comments / replies, so I'll reply here.

You can't wrap your fetch method (or otherwise augment the preload state) without actual sapper/sveltekit platform support.

This is because (on the server-side) the information you need to use while wrapping is hidden in the request headers.

So the minimum needed to "get around this" is:

in the same place.

benmccann commented 3 years ago

SvelteKit should be in beta shortly at which point you can comment on the issue. For now I've reopened the issue and left a comment there on your behalf

eikaramba commented 3 years ago

thanks, sry for my short answer i was exhausted. my use case is that i am using cookie based authentication which renders all solutions above (and more that i found and even tried) not applicable or at least i was not able to make it work (this.fetch automatically has the cookie based authentication so it would be great to just use that and give it as a fetcher for graphql). so i would appreciate any improvements here. i tend to be annoyed by the complexity of graphql and understand that SSR makes it even harder.