weixin_39713805
2021-01-12 19:11 阅读 10

Remove setContext / getContext from svelte.

Summary

see: https://github.com/FormidableLabs/urql/blob/main/packages/svelte-urql/src/context.ts

isolating setContext and getContext in a single file provides no benefit.

It does have a negative impact that could trip up svelte users.

this code for example wont run, as query has to be called synchronously during component initialisation. This could trip up a number of svelte users, and may also relate to some previous issues I've seen regarding SSR.

js
<script>
  import gql from 'graphql-tag';
  import { operationStore, query } from '/svelte';

  const todos = operationStore(gql`
    query ($first: Int!, $skip: Int) {
      todos(first: $first, skip: $skip) {
        id
      }
    }
  `, { first: 10 });

  onMount(() => {
     query(todos);
  }
</script>

Proposed Solution

Since we gain nothing by using setContext and getContext calls when isolated to the same file, I suggest context is changed to something along the lines of:

js
import { Client, ClientOptions } from '/core';

let client = new Client({ url: '/graphql' });

export const setClient = (args: ClientOptions): void => {
  client = new Client(args);
};
export const getClient = (): Client => client;

该提问来源于开源项目:FormidableLabs/urql

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享

5条回答 默认 最新

  • weixin_39601743 weixin_39601743 2021-01-12 19:11

    Sorry, I'll have to close this because I don't think the reasoning here becomes very clear.

    The context will have to be shared across multiple contexts to share the client. A global isn't acceptable actually in SSR scenarios (which currently barely work anyway afaik in Svelte) and removing utilities and mechanism around context which is doing exactly what's intended here doesn't fix any discernible issue in my eyes 😅

    Edit: I suppose what we could consider is throwing a more explicit error in development when our operations aren't called synchronously

    点赞 评论 复制链接分享
  • weixin_39713805 weixin_39713805 2021-01-12 19:11

    hmm, I'm not 100% sure I understand the issue your trying to solve, but remember this is not react so maybe same patterns don't apply ... ¯_(ツ)_/¯

    点赞 评论 复制链接分享
  • weixin_39601743 weixin_39601743 2021-01-12 19:11

    Ok, unfortunately, right back at'ya though 😅 You've opened an RFC but the problem statement isn't quite complete.

    All I currently get from this is that users may not synchronously call our operations like, query, which will fail pretty transparently. We can make this clearer with a nicer error but currently the docs do go through this and show how it's used.

    What I don't get though is how that'd relate to context. query is called synchronously because it's not just a function operating on a store but is supposed to follow the lifecycle and needs to unsubscribe from the client on unmount so nothing leaks.

    In fact it calls onDestroy internally, so even if it got the client directly it'd still not work as being called outside of the direct component code

    点赞 评论 复制链接分享
  • weixin_39713805 weixin_39713805 2021-01-12 19:11

    ok, but I'm pretty sure that setContextand getContext are litterally just passing the same global around - but anyway - some bits I'm not sure about - so don't want to swim any further out of my depth.

    点赞 评论 复制链接分享
  • weixin_39713805 weixin_39713805 2021-01-12 19:11

    ( correct me if I'm wrong)

    For anyone, who perhaps doesn't get it straight away (like me), using setContext/getContext will essentially allow your app to support more than one GraphQL client (different servers etc), a global wouldn't! It does unfortunately tie you into a certain pattern of where you intialise the query, but unavoidable.

    点赞 评论 复制链接分享

相关推荐