Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect types on fragments #2436

Closed
bwiklund opened this issue Aug 23, 2019 · 11 comments · Fixed by #2437
Closed

incorrect types on fragments #2436

bwiklund opened this issue Aug 23, 2019 · 11 comments · Fixed by #2437
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release

Comments

@bwiklund
Copy link

bwiklund commented Aug 23, 2019

we're getting incorrect types on some generated types, see below. I commented on the final snippet in the generated code where the types are seemingly wrong

type DashboardTileChartDetails {
  tileId: String!
  wsId: UUID!
  chartId: String!
  md: TileChartMetadata # null if we error out
}

type TileChartMetadata {
  connection: Connection!
  viz: JsonBlob!
  columnInfo: JsonBlob! # the ColumnInfo type;
}

type DashboardTileFilterDetails {
  tileId: String!
  md: TileFilterMetadata
}
const DashboardVersionFragment = gql`
  fragment DashboardVersionFragment on DashboardVersion {
    ...DocumentVersionFragment
    tiles {
      ... on DashboardTileChartDetails {
        tileId
        wsId
        chartId
        md {
          viz
          columnInfo
          connection {
            ...ConnectionFragment
          }
        }
      }
      ... on DashboardTileFilterDetails {
        tileId
        md {
          columnType
          connection {
            ...ConnectionFragment
          }
        }
      }
      ... on DashboardTileParameterDetails {
        tileId
        md {
          parameterConstraint
          parameterLabel
        }
      }
    }
  }
  ${DocumentVersionFragment}
  ${ConnectionFragment}
`;
export type DashboardVersionFragmentFragment = (
  { __typename?: 'DashboardVersion' }
  & { tiles: Array<(
    { __typename?: 'DashboardTileChartDetails' }
    & Pick<DashboardTileChartDetails, 'tileId' | 'wsId' | 'chartId'>
    & { md: Maybe<(
      { __typename?: 'TileChartMetadata' }
      & Pick<TileChartMetadata, 'viz' | 'columnInfo'>
      & { connection: { __typename?: 'Connection' }
        & ConnectionFragmentFragment
       }
    )> }
  ) | (
    { __typename?: 'DashboardTileFilterDetails' }
    & Pick<DashboardTileFilterDetails, 'tileId'>
    & { md: Maybe<(
      { __typename?: 'TileChartMetadata' }
      & Pick<TileChartMetadata, 'columnType'> // should pick TileFilterMetadata
      & { connection: { __typename?: 'Connection' }
        & ConnectionFragmentFragment
       }
    )> }
  ) | { __typename?: 'DashboardTileTextDetails' } | (
    { __typename?: 'DashboardTileParameterDetails' }
    & Pick<DashboardTileParameterDetails, 'tileId'>
    & { md: Maybe<(
      { __typename?: 'TileChartMetadata' }
      & Pick<TileChartMetadata, 'parameterConstraint' | 'parameterLabel'> // should pick TileParameterMetadata
    )> }
  )> }
)
  & DocumentVersionFragmentFragment
;
;
@n1ru4l
Copy link
Collaborator

n1ru4l commented Aug 23, 2019

Is the field tiles a union or interface? Please provide all referenced type definitions.

n1ru4l added a commit to n1ru4l/graphql-code-generator that referenced this issue Aug 23, 2019

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
n1ru4l added a commit to n1ru4l/graphql-code-generator that referenced this issue Aug 23, 2019

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@bwiklund
Copy link
Author

bwiklund commented Aug 23, 2019

sure here's some more context:

type DashboardVersion implements DocumentVersionInterface {
  version: Int!
  document: JsonBlob!
  description: String

  createdBy: OrgMember!
  updatedBy: OrgMember!

  createdAt: DateTime!
  updatedAt: DateTime!

  tiles: [DashboardTile!]!
}
union DashboardTile =
    DashboardTileChartDetails
  | DashboardTileFilterDetails
  | DashboardTileTextDetails
  | DashboardTileParameterDetails

@n1ru4l
Copy link
Collaborator

n1ru4l commented Aug 23, 2019

I also created a reproduction using interfaces instead of unions (see #2437)

@bwiklund
Copy link
Author

so this is confirmed to be a bug? i wanted to be sure we weren't discovering some issue with our schema through the type generation

n1ru4l added a commit to n1ru4l/graphql-code-generator that referenced this issue Aug 23, 2019

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@n1ru4l
Copy link
Collaborator

n1ru4l commented Aug 23, 2019

Yes it is a bug caused by some code that I forgot to delete when fixing union/interface types 😅

I fixed it in #2437

I added tests for both unions and interfaces to ensure there will be no regression!

@dotansimha dotansimha added bug plugins waiting-for-release Fixed/resolved, and waiting for the next stable release labels Aug 27, 2019
dotansimha pushed a commit that referenced this issue Aug 27, 2019

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
… set field name but different type (#2436) (#2437)

* test(typescript-operations): add failing test for #2436

* fix(visitor-plugin-common): remove code that caused #2436

* test(typescript-operations): add union test
@dotansimha dotansimha reopened this Aug 27, 2019
@tanguyantoine
Copy link

I'm experiencing this issue as well, thank you to have fixed it 👍

@dotansimha
Copy link
Owner

Fixed in 1.7.0
thanks @n1ru4l

@bwiklund
Copy link
Author

fwiw this is still broken in 1.8.2

@n1ru4l
Copy link
Collaborator

n1ru4l commented Oct 29, 2019

@bwiklund Could you please provide a code sandbox example or a failing test?

@dotansimha
Copy link
Owner

Just as @n1ru4l said, can you please share a reproduction of this? @bwiklund ?

@bwiklund
Copy link
Author

sorry, i dont have the bandwidth to recreate this entire environment for a repro. it's the same bug as original reported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release
Projects
None yet
4 participants