n1ru4l/graphql-live-query

InMemoryLiveQuery store doesn't respect previous `setExecuteFn`s by other envelop plugins

Closed this issue · 2 comments

Hey there! Really appreciate you and your teams work on this library and envelop as well.

My envelop setup is as follows:

  const getLiveEnveloped = envelop({
    plugins: [
      // order matters because of the order of execute wrapping
      useSchema(schema),
      useClientFromPgSettings(pg),
      useLiveQuery({ liveQueryStore }),
      ...moreIrrelevantPlugisn
    ],
  });

My useClientFromPgSettings plugin is an envlop onExecute hook, that looks something like this, and is responsible for creating and managing connections to the database available to all resolvers:

  return {
    onExecute: async ({ extendContext, args, executeFn, setExecuteFn }) => {
      const pgSettings = args.contextValue.pgSettings;
      const { setup, teardown } = makeCreateAndTeardownPostgresClient()

      const client = await setup();

      await attachPgSettingsToClient(resolvedPgSettings, client);

      extendContext({
        pgClient: client,
      });

      const useClientWithSettingsExecuteFn = async (args) => {
        const result = await executeFn(args);
        await teardown();
        return result;
      };

      setExecuteFn(useClientWithSettingsExecuteFn);
    },
  }

However, when liveQueryStore.execute runs, it calls the default execute provided when I created a new InMemoryLiveQueryStore.

That execute (the one provided in params) is the one set to this._execute, and the execute returning by the envlop plugins mounted before useLiveQuery is the parameter passed to makeExecute here: https://github.com/n1ru4l/graphql-live-query/blob/main/packages/in-memory-live-query-store/src/InMemoryLiveQueryStore.ts#L229

InMemoryLiveQuery store will actually only use the envlop orchestrator's executeFn if it's not a live query.

I cannot pass the execute function created inside of useClientFromPgSettings because it doesn't exist when the InMemoryLiveQueryStore needs to be initialized.

Would it make more sense to use the execute passed to makeExecute as the internal call that the store refreshes on invalidate in order to support the programming style promoted by envelop?

Happy to make this PR myself, just want to check that there's not some reason I don't understand why this is a bad idea

@ben-pr-p You spotted a bug! Would appreciate a PR! Indeed the this._execute call should use the execute passed to makeExecute instead!