-
Notifications
You must be signed in to change notification settings - Fork 2.7k
remove react/context|hooks|parser entry points, remove parser utils completely
#12513
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
Conversation
🦋 Changeset detectedLatest commit: 19ef2c9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
commit: |
size-limit report 📦
|
40999b8 to
d176cbd
Compare
98ade11 to
d3dc528
Compare
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| invariant( | ||
| !!document && !!document.kind, | ||
| `Argument of %s passed to parser was not a valid GraphQL ` + | ||
| `DocumentNode. You may need to use 'graphql-tag' or another method ` + | ||
| `to convert your operation into a document`, | ||
| document | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Covered by
apollo-client/src/utilities/graphql/getFromAST.ts
Lines 21 to 25 in ca9089b
| invariant( | |
| doc && doc.kind === "Document", | |
| `Expecting a parsed GraphQL document. Perhaps you need to wrap the query \ | |
| string in a "gql" tag? http://docs.apollostack.com/apollo-client/core.html#gql` | |
| ); |
| invariant( | ||
| queries.length + mutations.length + subscriptions.length <= 1, | ||
| `react-apollo only supports a query, subscription, or a mutation per HOC. ` + | ||
| `%s had %s queries, %s ` + | ||
| `subscriptions and %s mutations. ` + | ||
| `You can use 'compose' to join multiple operation types to a component`, | ||
| document, | ||
| queries.length, | ||
| subscriptions.length, | ||
| mutations.length | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Covered by
apollo-client/src/utilities/graphql/getFromAST.ts
Lines 39 to 43 in ca9089b
| invariant( | |
| operations.length <= 1, | |
| `Ambiguous GraphQL document: contains %s operations`, | |
| operations.length | |
| ); |
| invariant( | ||
| definitions.length === 1, | ||
| `react-apollo only supports one definition per HOC. %s had ` + | ||
| `%s definitions. ` + | ||
| `You can use 'compose' to join multiple operation types to a component`, | ||
| document, | ||
| definitions.length | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also covered by
apollo-client/src/utilities/graphql/getFromAST.ts
Lines 39 to 43 in ca9089b
| invariant( | |
| operations.length <= 1, | |
| `Ambiguous GraphQL document: contains %s operations`, | |
| operations.length | |
| ); |
react/context|hooks|parser entry points, move verifyDocumentType to utilitiesreact/context|hooks|parser entry points, remove parser utils completely
| invariant( | ||
| operation.type === type, | ||
| `Running a %s requires a graphql ` + `%s, but a %s was used instead.`, | ||
| requiredOperationName, | ||
| requiredOperationName, | ||
| usedOperationName | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a new optional check here:
| if (expectedType) { |
| > | ||
| ): useMutation.ResultTuple<TData, TVariables, TContext, TCache> { | ||
| const client = useApolloClient(options?.client); | ||
| verifyDocumentType(mutation, DocumentType.Mutation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These checks all moved into QueryManager, and since they now are not executed on every render, but only if an operation is started, they also don't need caching.
| "operationName", | ||
| "parser", | ||
| "useApolloClient", | ||
| "useBackgroundQuery", | ||
| "useFragment", | ||
| "useLazyQuery", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diff is not really nice, it looks like "removed" here and then merges in a removed entry point with the same exports further down.
|
|
||
| const { hasOwnProperty } = Object.prototype; | ||
|
|
||
| // TODO(brian): A hack until this issue is resolved (https://github.com/graphql/graphql-js/issues/3356) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means we have to drop GraphQL 15 compat, which we probably should do anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| declare namespace ApolloConsumer { | ||
| export interface Props { | ||
| children: (client: ApolloClient) => ReactTypes.ReactNode; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was previously exported from /react/context, but not from /react. I don't want to add more noise to /react, and this keeps it available for use from the outside.
Same for ApolloProvider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be called out in the changelog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh., I don't think anyone ever used those - it would probably be more noise than it would be worth.
| } from "@apollo/client/core"; | ||
| import { createPersistedQueryLink } from "@apollo/client/link/persisted-queries"; | ||
| import { removeTypenameFromVariables } from "@apollo/client/link/remove-typename"; | ||
| // importing react so the `parser` cache initializes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice to see this gone 🎉
|
|
||
| // Checks the document for errors and throws an exception if there is an error. | ||
| export function checkDocument(doc: DocumentNode) { | ||
| export function checkDocument( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a good candidate to move to utilities/internal. Adding this as a reminder to do in #12194
jerelmiller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
…