Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Feature request: Preload argument for context #751

Closed
hitsthings opened this issue Jun 18, 2019 · 15 comments
Closed

Feature request: Preload argument for context #751

hitsthings opened this issue Jun 18, 2019 · 15 comments
Labels

Comments

@hitsthings
Copy link
Contributor

hitsthings commented Jun 18, 2019

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:

  • setting up a shared ApolloClient instance for use both during preload and normal component lifecycle.
    • I can't use context to pass it because it's not available during preload.
    • I can't use session to pass it because the server-side version would be serialized to the client.
    • On the client I could just have a singleton in a JS module. But I can't do that on the server because then every request would share the same ApolloClient (bad news). I also need access to the req object on the server so I can forward cookies.
  • setting up honeycomb.io integration to send events when there is an error
    • on the client, I serialize events and hit a server route which calls honeycomb-beeline for each one (honecomb-beeline is their official client)
    • on the server, the events are created directly against honeycomb-beeline

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
Copy link
Contributor Author

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.

hitsthings added a commit to hitsthings/sapper that referenced this issue Jun 25, 2019
…ent.js

This is a preliminary commit for supporting root-level context on server and client.

It currently does _too_ much and I need feedback on how to proceed. Here's what it currently does.

In client.js, you can now do:

```js
sapper.start({
    target: ...,
    context: () => {
        setContext('key', new Blammo()); // set root-level context

        // can also return a Map to merge into the context.
        // I don't think both APIs should be supported, but they currently are.
        // Which one do you prefer?
        return new Map([['key2', new Foobar()]]);
    }
})
```

In `server.js` you can do:

```
middleware({
    context: (req, res) => {
        setContext('key', new Blammo()); // set root-level context

        // can also return a Map to merge into the context.
        // I don't think both APIs should be supported, but they currently are.
        // Which one do you prefer?
        return new Map([['key2', new Foobar()]]);
    }
})
```

In `preload` there is both a new argument _and_ support for `getContext` and `setContext`. I don't think we should support both.

```js
export async function preload(page, session, context) {
    const foobar = context.get('key2'); // can access the context initialized above.
    const blammo = getContext('key'); // again - we shouldn't support both of these.
}
```

The code for supporting `getContext` and `setContext` is also a bit dirty for my liking, and there are TODOs in there about it. Is there a better way than calling `set_current_component`?

Further works needs to be done before this is can be merged.
@utherpally
Copy link

utherpally commented Jul 31, 2019

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
Copy link

ajbouh commented Sep 30, 2019

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
Copy link

justinmoon commented Jan 2, 2020

This has been my biggest issue with Sapper so far

@simoncpu
Copy link

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
Copy link

ajbouh commented May 22, 2020

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
Copy link

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

@benmccann
Copy link
Member

@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
Copy link
Member

bluwy commented Dec 17, 2020

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
Copy link

ajbouh commented Dec 18, 2020

This is a nice approach!

@benmccann
Copy link
Member

Closing as duplicate of sveltejs/kit#327

@eikaramba
Copy link

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

@ajbouh
Copy link

ajbouh commented Mar 23, 2021

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:

  • access to request headers and
  • access to the values passed to load/prefetch

in the same place.

@benmccann
Copy link
Member

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
Copy link

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.

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

No branches or pull requests

8 participants