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

CRA migration failed: unknown graphql fragments (macro issue?) #5861

Closed
bebbi opened this issue Nov 20, 2018 · 4 comments
Closed

CRA migration failed: unknown graphql fragments (macro issue?) #5861

bebbi opened this issue Nov 20, 2018 · 4 comments

Comments

@bebbi
Copy link
Contributor

bebbi commented Nov 20, 2018

Is this a bug report?

Yes.

After migrating from 2.0.0-next.66cc7a90 to 2.1.1, graphql queries do not work at runtime.
The (unaffected) graphql api returns 400, and its body complains (per examples):

"body":"{\"errors\":[{\"message\":\"Unknown fragment \\\"OwnerScalarsFragment\\\".\",\"code\":500}]}"

When playing, I noticed it works well for queries that import a fragment, but not for queries that import fragments that import fragments

Did you try recovering your dependencies?

Yes.

Which terms did you search for in User Guide?

graphql, graphql.macro, fragments

Related are: #5580 , #5076

Environment

A monorepo with

OS: macOS 10.14.1
Node: 8.12.0
Yarn: 1.7.0
npm: 6.4.1
Watchman: 4.9.0
Xcode: Xcode 10.1 Build version 10B61
Android Studio: Not Found

Packages: (wanted => installed)
react: ^16.4.0 => Not Installed
react-dom: ^16.4.0 => Not Installed
react-scripts: ^2.1.1 => 2.1.1

Packages: (wanted => installed)
react: ^16.4.1 => 16.4.1
react-dom: ^16.4.1 => 16.4.1
react-scripts: ^2.0.0-next.3e165448 => 2.0.0-next.3e165448

Steps to Reproduce

This is the sample app:

import React from 'react'
import { Query } from 'react-apollo'
import { ApolloProvider, client } from './apolloSetup'
import { loader as gqlLoader } from 'graphql.macro';
const GET_HISTORY = gqlLoader('./GetHistoryQuery.graphql')

const GetHistoryQuery = ({ children, id }) => (
  <Query query={GET_HISTORY} variables={{ id }}>
    {({ data = {}, error, loading }) => {
      const { history } = data
      return children({ history, error, loading })
    }}
  </Query>
)

const id = 'myId'

export default () => (
  <ApolloProvider client={client}>
    <GetHistoryQuery id={id}>
      {({ history, loading, error }) => {
        if (loading) {
          return <p>{loading}</p>
        }
        if (error) {
          throw new Error(error)
        }
        return <p>{history.id}</p>
      }}
    </GetHistoryQuery>
  </ApolloProvider>
)

GetHistoryQuery.graphql:

#import "./HistoryScalarsFragment.graphql"

query getHistory($id: ID!) {
  history(id: $id) {
    ...HistoryScalarsFragment
  }
}

HistoryScalarsFragment.graphql:

#import "./OwnerScalarsFragment.graphql"

fragment HistoryScalarsFragment on History {
  __typename
  id
  name
  owner {
    ...OwnerScalarsFragment
  }
}

OwnerScalarsFragment.graphql:

fragment OwnerScalarsFragment on User {
  __typename
  id
  name
  picture
}

In HistoryScalarsFragment, replacing ...OwnerScalarsFragment with the fragment's content (and then changing the app because of #5580 ) fixes the error mentioned on top.

Expected Behaviour

Since we had #5076, I would have expected the migration instructions for graphql to lead to no difference in my app's behaviour, or contain info on issues to expect and workarounds.

Actual Behaviour

The issue is that the app sends the HistoryScalarsFragment but not the OwnerScalarsFragment to the backend which then errors. This can be confirmed in the network tab. It looks like an issue with graphql.macro?

At least once in 20 tries, when I created a new fragment from scratch I believe have gotten 1 level of indirection to work, but not afterwards. So it could somehow relate to #5580, but then, other tries at creating fresh .graphql files and then referencing them all lead to errors like the one mention on top.

It would be nice to fix this or highlight alternatives (and other issues like #5580) in the migration documentation or the user guide.

Maybe we could even revert #5076 :D until there's a simpler way to import .graphql files.

@bebbi
Copy link
Contributor Author

bebbi commented Nov 20, 2018

Ok so it's not CRA, but then, graphql.macro is not a practical suggestion due to multiple issues, in the macro and #5580.

I think it shouldn't be the proposed way in the 2 alpha migration guide so keeping this issue open.
Maybe #5076 could be deferred till a better solution is available? #4893 seemed to make a strong but not urgent case..
cc /@Timer

@Timer
Copy link
Contributor

Timer commented Nov 20, 2018

It sounds like the best path forward is to just fix the macro. And #5580 is being actively worked on / considered.

@stale
Copy link

stale bot commented Dec 20, 2018

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Dec 20, 2018
@stale
Copy link

stale bot commented Dec 26, 2018

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.

@stale stale bot closed this as completed Dec 26, 2018
@lock lock bot locked and limited conversation to collaborators Jan 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants