Giter Club home page Giter Club logo

Comments (28)

olso avatar olso commented on July 24, 2024 9

Little dirty, but works for my use case

// https://github.com/relay-tools/relay-compiler-language-typescript/issues/64#issuecomment-511765083
type NoRefs<T> = T extends object ? Omit<T, " $refType" | " $fragmentRefs"> : T;

// https://www.typescriptlang.org/docs/handbook/advanced-types.html#type-inference-in-conditional-types
// https://stackoverflow.com/questions/43537520/how-do-i-extract-a-type-from-an-array-in-typescript#comment94888844_52331580
type Unpacked<T> = T extends Array<infer U> ? U : T extends ReadonlyArray<infer U> ? U : T;

type ExtractRelayEdgeNode<T> = NoRefs<
  NonNullable<NonNullable<Unpacked<NonNullable<NonNullable<T>["edges"]>>>["node"]>
>;

and use it like

type EdgeNode = ExtractRelayEdgeNode<SomeEdges_list>;
type EdgeNode = ExtractRelayEdgeNode<SomeEdges_list["search"]>;

from relay-compiler-language-typescript.

tomconroy avatar tomconroy commented on July 24, 2024 6

Hello! We have a strong use-case for this kinda thing, especially because our schema is mostly nullable types, and we have strictNullChecks. So to get to anything reasonably deep, it's necessary to do stuff like this:

type RecircNode = NonNullable<
  NonNullable<
    NonNullable<
      NonNullable<
        NonNullable<VideoPlayerNextUpData_viewer['realmContent']>['recircForNode']
      >['edges']
    >[0]
  >['node']
>;

type RecircSeriesMember = NonNullable<
  NonNullable<NonNullable<RecircNode['members']>['edges']>[0]
>['node'];

It would be lovely to be able to add something like @generateType(name: "RecircNode") and get these types for free

from relay-compiler-language-typescript.

josephsavona avatar josephsavona commented on July 24, 2024 4

The major problem I see with trying to add such a feature is naming. In a single fragment you could be selecting different fields of the same type at various levels in the graph, because of that you can't just name a selection set after the type, but you'd have to somehow make them unique. By using fragments you get to control at what level you actually want this separation and how to name things.

This. I strongly agree with your conclusion for the exact reasons you listed. We highly recommend using fragments as a way to describe a unit of data that you want to access.

from relay-compiler-language-typescript.

bitofbreeze avatar bitofbreeze commented on July 24, 2024 4

@olso I get

Type '"node"' cannot be used to index type 'NonNullable<Unpacked<NonNullable<NonNullable<T>["edges"]>>>'

in typescript 3.8.3

from relay-compiler-language-typescript.

alloy avatar alloy commented on July 24, 2024 3

I actually have come around and think that using fragments to split these up is the way it should be done, as per @kastermester's original comments.

The major problem I see with trying to add such a feature is naming. In a single fragment you could be selecting different fields of the same type at various levels in the graph, because of that you can't just name a selection set after the type, but you'd have to somehow make them unique. By using fragments you get to control at what level you actually want this separation and how to name things.

As for the fragment reference, I'm not sure what the upstream Flow emission does, but it seems to me that if a fragment is defined with the @relay(mask: false) directive there isn't any need for us to emit the opaque fragment references, so that's something that we could perhaps change to improve the experience.

@jstejada @josephsavona do you have any thoughts on this?

from relay-compiler-language-typescript.

josephsavona avatar josephsavona commented on July 24, 2024 3

Agreed though - now that we have @inline we should recommend that for utility functions, and may not need the separate types for each part of a fragment.

from relay-compiler-language-typescript.

alloy avatar alloy commented on July 24, 2024 2

An even more compelling argument could be made for unions, as there’s no way (that I know of?) to get a reference to a single type in a union with just TS.

from relay-compiler-language-typescript.

artola avatar artola commented on July 24, 2024 2

@alloy @sibelius could you please refresh the status of this issue? As you stated at the beginning, separate types for each interface is something more than nice to have.
In my case, trying to get the types of a deep leaf, is a no go type X = mybag[0]['transactions']['edges'][0]['node']; ... on top, some are nullables.

from relay-compiler-language-typescript.

artola avatar artola commented on July 24, 2024 2

@tomconroy I am with you, it would be nice to get the types split in parts (apollo's way).

In the meantime, just a hacky trick for similar case than yours, create a fragment of the node, and then spread it in the position needed. In this way I get the typings for that fragment.

e.g.

// issues_node.ts
import {graphql} from 'react-relay';

// tslint:disable-next-line: no-unused-expression
graphql`
  fragment issuesNode on Issue @relay(mask: false) {
    id
    title
    repository {
      name
    }
    viewerDidAuthor
    state
  }
`;

==> generates

// issuesNode.graphql.ts
import { ConcreteFragment } from "relay-runtime";
export type IssueState = "CLOSED" | "OPEN" | "%future added value";
declare const _issuesNode$ref: unique symbol;
export type issuesNode$ref = typeof _issuesNode$ref;
export type issuesNode = {
    readonly id: string;
    readonly title: string;
    readonly repository: {
        readonly name: string;
    };
    readonly viewerDidAuthor: boolean;
    readonly state: IssueState;
    readonly " $refType": issuesNode$ref;
};

Now we are a way closer, excepts the extra $refType, possible to remove with:

/**
 * Removes recursively all traces of `$fragmentRefs` and `$refType` from type.
 * @see https://github.com/relay-tools/relay-compiler-language-typescript/issues/29#issuecomment-417267049
 */
type NoFragmentRefs<T> = T extends object
  ? {
      [P in Exclude<keyof T, ' $fragmentRefs' | ' $refType'>]: NoFragmentRefs<
        T[P]
      >
    }
  : T;

from relay-compiler-language-typescript.

olso avatar olso commented on July 24, 2024 2

@CSFlorin

Just hit it as well, this seems to work for me with typescript 4.0.1

// https://github.com/relay-tools/relay-compiler-language-typescript/issues/64#issuecomment-511765083
export type NoRefs<T> = T extends Record<string, unknown>
  ? Omit<T, " $refType" | " $fragmentRefs">
  : T;

export type ExtractRelayEdgeNode<
  T extends { edges: ReadonlyArray<{ node: any | null }> | null } | null
> = NoRefs<NonNullable<NonNullable<NonNullable<NonNullable<T>["edges"]>[0]>["node"]>>;

from relay-compiler-language-typescript.

kastermester avatar kastermester commented on July 24, 2024 1

Without having tried this, does the type checking work if you move the directive to the fragment itself (not when it is being spread) ie.

graphql`fragment ShippingAndPaymentReview_orderAddressDetails on Order @relay(mask: false) {
    shippingName
    shippingAddressLine1
    shippingAddressLine2
    shippingCity
    shippingPostalCode
    shippingRegion
  }

It was my understanding that this should work (from reading the docs) but I have not yet tried this myself.

from relay-compiler-language-typescript.

dotbear avatar dotbear commented on July 24, 2024 1

So I originally found this issue because I needed to send parts of the type to a function to do work with it (similar to above), however this was successfully solved using fragments like suggested (yay).

However now I've run into the union problem - did anyone solve this properly? I'm rendering a form and showing certain fields based on what type of product is being edited, while most fields are shared between the types (showing shipping settings for a physical product, but file settings for a digital one). I'm not really sure how to solve this?

I'd preferably like to use a type guard

(product: Product): product is PhysicalProduct => product.__typename === "PhysicalProduct"

however I don't know how I would derive types Product, PhysicalProduct, and DigitalProduct here using relay-compiler-language-typescript.

Here is my example query

product(uuid: $uuid) {
  __typename
  name
  summary
  description

  ... on PhysicalProduct {
    estimatedProcessingTime
    packagingType
  }

  ... on DigitalProduct {
    files { [...] }
  }
}

Any thoughts?

from relay-compiler-language-typescript.

kastermester avatar kastermester commented on July 24, 2024

We have run into the same issue at our (still sigh) legacy based solution. The issue is that there's really not a great way to name these types.

I think the intent behind relay is to have a named fragment (not used directly within a container) and then spread it using @relay(mask: false) define it using @relay(mask: false). But perhaps this is an issue we should take up directly with the relay team?

See: https://facebook.github.io/relay/docs/en/graphql-in-relay.html#relaymask-boolean

from relay-compiler-language-typescript.

kastermester avatar kastermester commented on July 24, 2024

FYI we solve this, somewhat, in our old solution using directives inside the GraphQL fragment type, ie.

fragment on MyType {
  name
  profile @generateType(name: "Profile") {
     id
     name
  }
}

This still does not solve the union issue though, and it is also a bit complicated when the given type is a list type.

from relay-compiler-language-typescript.

ds300 avatar ds300 commented on July 24, 2024

there’s no way (that I know of?) to get a reference to a single type in a union with just TS.

It is possible to reference elements of a union using Extract

http://www.typescriptlang.org/play/#src=type%20Union%20%3D%20%7B%20type%3A%20%22FOO%22%3B%20foo%3A%20true%20%7D%20%7C%20%7B%20type%3A%20%22BAR%22%3B%20bar%3A%20true%20%7D%0D%0A%0D%0Atype%20Foo%20%3D%20Extract%3CUnion%2C%20%7Btype%3A%20%22FOO%22%7D%3E%20%0D%0A%0D%0Adeclare%20const%20x%3A%20Foo%0D%0A%0D%0Ax.foo

from relay-compiler-language-typescript.

alloy avatar alloy commented on July 24, 2024

@ds300 Ooh nice one, didn’t know that one exist yet! And it’s so trivial in retrospect: type Extract<T, U> = T extends U ? T : never;

from relay-compiler-language-typescript.

alloy avatar alloy commented on July 24, 2024

@kastermester Oh interesting thought! Yeah, that technically works.

Although I do think that means we need to be careful that the processed AST from relay-compiler doesn’t actually get required at runtime and added to the JS bundle generated by e.g. webpack. An easy fix would be for the fragment to exist in a separate module that never actually gets required anywhere, but I don’t like how that breaks the co-location aspect 🤔


Currently, however, this is breaking on our emitted fragment reference type checking. Consider the following example:

export const ShippingAndPaymentReviewFragmentContainer = createFragmentContainer(
  ShippingAndPaymentReview,
  graphql`
    fragment ShippingAndPaymentReview_order on Order {
      fulfillmentType
      shippingName
      shippingAddressLine1
      shippingAddressLine2
      shippingCity
      shippingPostalCode
      shippingRegion
      lineItems {
        edges {
          node {
            artwork {
              shippingOrigin
            }
          }
        }
      }
      creditCard {
        brand
        last_digits
        expiration_year
        expiration_month
      }
    }
  `
)

...which gets split up into:

graphql`
  fragment ShippingAndPaymentReview_orderAddressDetails on Order {
    shippingName
    shippingAddressLine1
    shippingAddressLine2
    shippingCity
    shippingPostalCode
    shippingRegion
  }
`

export const ShippingAndPaymentReviewFragmentContainer = createFragmentContainer(
  ShippingAndPaymentReview,
  graphql`
    fragment ShippingAndPaymentReview_order on Order {
      fulfillmentType
      ...ShippingAndPaymentReview_orderAddressDetails @relay(mask: false)
      lineItems {
        edges {
          node {
            artwork {
              shippingOrigin
            }
          }
        }
      }
      creditCard {
        brand
        last_digits
        expiration_year
        expiration_month
      }
    }
  `
)

In this case the emitted typings look like:

export type ShippingAndPaymentReview_orderAddressDetails = {
    // ...
    readonly " $refType": ShippingAndPaymentReview_orderAddressDetails$ref;
};

export type ShippingAndPaymentReview_order = {
    readonly fulfillmentType: OrderFulfillmentType | null;
    readonly shippingName: string | null;
    // ...
};

For the data to be assignable to the address details type, the parent should include " $fragmentRefs": ShippingAndPaymentReview_orderAddressDetails$ref regardless of its data not being masked.

from relay-compiler-language-typescript.

alloy avatar alloy commented on July 24, 2024

@kastermester The custom directive is an interesting alternative and simplifies some of these things, just feels a bit less clean to mix such details into the fragment. Although OTOH @relay(mask: false) in this example also doesn’t nicely convey the semantics of its usage 🤔

So yeah, might be something to talk to the Relay team about and also check if the emitted Flow types do work in the @relay(mask: false) scenario.

from relay-compiler-language-typescript.

kastermester avatar kastermester commented on July 24, 2024

Oh also - the fragment ref is, the way I understand it, not supposed to be included in the fragment refs. The idea is not to use the @relay(mask: false) when you want to render a component that requires some data - it is a way of embedding a fragment inside your component while you want access to the data only inside that component. I think if you want both (access to the data + rendering another component with that same data) you should do one of the following:

  1. Define two different fragments - possibly requiring the same data, and define one using @relay(mask: false) and one without. Use the non-masked one inside the parent component and the other one inside the child component.
  2. Define only one and spread it twice, once with mask false and once without the directive. I don't think this one plays as nicely for most use cases however. One should be really careful using the directive for this.

Directives with @relay(mask: false) to me really reads "copy&paste the fragment data in here into the query". Ie. you should not see it as "I want to render another component using its fragment" but rather "for some reason I do not want to type out the data I need here, so I'm just referring to it using this fragment name". In this case that reason is simply that you want a name for that piece of data.

from relay-compiler-language-typescript.

kastermester avatar kastermester commented on July 24, 2024

One last thing:

One thing that should work however is that if we have the following fragments:

fragment MyComponent_nonMaskedFragment on SomeType @relay(mask: false) {
   id
   otherType {
     ...OtherComponent_prop
   }
}

fragment MyComponent_prop on SomeType {
  ...MyComponent_nonMaskedFragment
  id
}

fragment OtherComponent_prop on SomeOtherType {
  id
}

Obviously using this - we need to be able to render OtherComponent using the data from MyComponent_prop. We probably need some tests around this (if they are not already there somehow from the tests we copied from the official repo). I would expect the type to be something like:

export type MyComponent_nonMaskedFragment = {
    // Not sure if this will be here or not:
    readonly " $refType": MyComponent_nonMaskedFragment$ref;
    // ...
    readonly id: string;
    readonly otherType: {
      readonly " $fragmentRefs": OtherComponent_prop$ref;
    };
    // ...
};

export type MyComponent_prop = {
    readonly " $refType": MyComponent_Prop$ref;
    // ...
    readonly id: string;
    readonly otherType: {
      readonly " $fragmentRefs": OtherComponent_prop$ref;
    }
    // ...
};

Does this make sense?

from relay-compiler-language-typescript.

alloy avatar alloy commented on July 24, 2024

@josephsavona Thanks 🙏

from relay-compiler-language-typescript.

tomconroy avatar tomconroy commented on July 24, 2024

@josephsavona @alloy thank you for the feedback, this makes sense. I did go down this route, trying to use @relay(mask: false) but ran into this issue: facebook/relay#2118. It's impossible to have arguments inside such a fragment? If that issue gets fixed then named fragments are a great solution

from relay-compiler-language-typescript.

dotbear avatar dotbear commented on July 24, 2024

Hm, I suppose maybe I could @relay(mask: false) the PhysicalProduct and DigitalProduct parts, as well as the common query parts which would get me Common, PhysicalProduct, and DigitalProduct thus allowing me to define

type FullPhysicalProduct = Common & PhysicalProduct
type FullDigitalProduct = Common & DigitalProduct

Maybe?

from relay-compiler-language-typescript.

dotbear avatar dotbear commented on July 24, 2024

Given the following root query in my QueryRenderer:

product(uuid: $uuid) {
  ...EditProduct_product @relay(mask: false)
}

I then get

graphql`
  fragment EditProduct_product on Product @relay(mask: false) {
    ...EditProduct_common @relay(mask: false)

    ... on MerchProduct {
      ...EditProduct_merch @relay(mask: false)
    }
  }
`;

graphql`
  fragment EditProduct_merch on MerchProduct @relay(mask: false) {
    __typename
    estimatedProcessingTime
    packagingType
  }
`;

graphql`
  fragment EditProduct_common on Product @relay(mask: false) {
    __typename
    name
    summary
    description
  }
`;

export type NoRefs<T> = T extends object ? Omit<T, " $refType" | " $fragmentRefs"> : T;

export type MerchProduct = NoRefs<EditProduct_common> & NoRefs<EditProduct_merch>;

export const isMerchProduct = (product: NoRefs<EditProduct_product>): product is MerchProduct =>
  product.__typename === "MerchProduct";

Here is what I ended up doing - in case others are looking for a solution

from relay-compiler-language-typescript.

maraisr avatar maraisr commented on July 24, 2024

Can I just interject (and probably a side question); what is the use-case for mask: false, in what use case would this be a good idea? I mean, like what stops a spread from changing fields, and then breaking upstream? I thought the whole idea was to declare data requirements for a particular thing. And if you want to read things that is masked, make a new fragment out of it, place it in a readInlineData, and be safe there also.

from relay-compiler-language-typescript.

josephsavona avatar josephsavona commented on July 24, 2024

@maraisr The use-case is things like utility functions that are not executing in a React context and therefore don't have access to the context's environment. @relay(mask:false) was our earlier solution to this (which as you noted has some issues), @inline is its replacement.

from relay-compiler-language-typescript.

alloy avatar alloy commented on July 24, 2024

Yeah, seems fair to close this by now 👍

from relay-compiler-language-typescript.

RichardLindhout avatar RichardLindhout commented on July 24, 2024

@olso solutions worked for me but I needed one extra null of the edge node

// https://github.com/relay-tools/relay-compiler-language-typescript/issues/64#issuecomment-511765083
export type NoRefs<T> = T extends Record<string, unknown>
  ? Omit<T, ' $refType' | ' $fragmentRefs'>
  : T;

export type ExtractRelayEdgeNode<
  T extends { edges: ReadonlyArray<{ node: any | null } | null> | null } | null
> = NoRefs<
  NonNullable<NonNullable<NonNullable<NonNullable<T>['edges']>[0]>['node']>
>;

from relay-compiler-language-typescript.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.