add NormalizationAstLoader

[PatrykWalach]

#297 should be merged first

Part of #33, #43

[PatrykWalach]

Overloading makes an ugly error message if ast is lazy loaded then for useLazyReference(entrypoint, {}) the error will be NormalizationAstLoader is not assignable to NormalizationAst, instead of missing argument error.

[rbalicki2]

🤔

[rbalicki2]

partial review

[rbalicki2]

I love that if we have an entrypoint with an AST loader, we prevent IfNecessary at the type level for useLazyReference, can we do the same here?

That being said, (eventually?) you should be able to turn an Entrypoint<..., NormalizationAstLoader> into a Promise<Entrypoint<..., NormalizationAst>> and then load that second entrypoint with IfNecessary, if you choose to. So, you should be able to (with some extra work, and by being explicit about it) achieve what you're erroring out on

[rbalicki2]

I get pretty good (?) type errors with

export type FetchOptions<
  TReadOutData,
  NormalizationAstLoaded extends boolean,
> = {
  onComplete?: (data: TReadOutData) => void;
  onError?: () => void;
} & (NormalizationAstLoaded extends true
  ? {
      shouldFetch?: ShouldFetch;
    }
  : {
      shouldFetch: RequiredShouldFetch;
    });

export type IsographEntrypoint<
  TReadFromStore extends { parameters: object; data: object },
  TClientFieldValue,
  NormalizationAstLoaded extends boolean,
> = {
  readonly kind: 'Entrypoint';
  readonly networkRequestInfo: NetworkRequestInfo<
    NormalizationAstLoaded extends true
      ? NormalizationAst
      : NormalizationAstLoader
  >;
  readonly readerWithRefetchQueries: ReaderWithRefetchQueries<
    TReadFromStore,
    TClientFieldValue
  >;
  readonly concreteType: TypeName;
};

[rbalicki2]

Along with (with no overloads)

export function useLazyReference<
  TReadFromStore extends { parameters: object; data: object },
  TClientFieldValue,
  NormalizationAstLoaded extends boolean,
>(
  entrypoint: IsographEntrypoint<
    TReadFromStore,
    TClientFieldValue,
    NormalizationAstLoaded
  >,
  variables: ExtractParameters<TReadFromStore>,
  fetchOptions?: FetchOptions<TClientFieldValue, NormalizationAstLoaded>,
): {
  fragmentReference: FragmentReference<TReadFromStore, TClientFieldValue>;
} {

[PatrykWalach]

But then the third argument is optional so you only get an error if you provide the third argument.

[rbalicki2]

Oh I guess we do need the overload then

[rbalicki2]

Okay, I can't obviously see a way to improve upon your approach. Maybe we might have cleaner type errors if we have two separate type names IsographEntrypoint and IsographEntrypointWithNormalizationAstLoader, but anyway, that is something we can play with after getting the functionality landed

[rbalicki2]

actually, I'm going back and forth. I do think it is nicer to have everything parametrized by a boolean (which could be "NormalizedAstPresent" | "AsyncNormalizationAst" or something for more clarity), even though we have to keep the overloads

I'm also not opposed to making the third arg not optional, as silly as it would look

[rbalicki2]

check out this ts playground by lenz

I haven't (yet) looked super closely into it, but perhaps we can improve on the error messages using some of the techniques there

[rbalicki2]

🤔

[rbalicki2]

just a nit plus something to investigate

I'll look at that as IsographEntrypoint thing as well, and this should be good to land

[rbalicki2]

LGTM overall. It might make sense to try to avoid this by not having IsographEntrypoint<T1, T2> but having IsographEntrypoint<T1, T2, NormalizationAst> in various places, or changing the type of maybeMakeNetworkRequest.

I don't know if that's possible.

[rbalicki2]

TNormalizationAst extends NormalizationAst | NormalizationAstLoader = NormalizationAst | NormalizationAstLoader

This way you can't instantiate this with anything unrelated