-
Notifications
You must be signed in to change notification settings - Fork 257
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
Fix TS / runtime errors while using graphql@15
#1482
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
GraphQL v15 Support: | ||
docker: | ||
- image: cimg/node:16.0.0 | ||
steps: | ||
- oss/install_specific_npm_version: | ||
version: '7' | ||
- checkout | ||
- run: | ||
command: npm i -D graphql@15.8.0 | ||
- run: | ||
command: npm run clean && npm i | ||
- run: | ||
command: npm run test:ci | ||
environment: | ||
JEST_JUNIT_OUTPUT_DIR: "reports/junit/" | ||
- store_test_results: | ||
path: reports/junit | ||
- store_artifacts: | ||
path: reports/junit |
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.
@abernix I'd be curious to hear if you have any opinions on this. We've run into a couple v15 compat issues, this particular version straddle has a few gotchas. Seems like a reasonable way to catch this in CI.
I found the npm run clean && npm i
to work around a problem where it seemed the TS build was out of date (probably was via the caching, which this breaks/works around). Maybe there's a better way that's still cacheable?
subgraph-js/package.json
Outdated
@@ -23,7 +23,10 @@ | |||
"publishConfig": { | |||
"access": "public" | |||
}, | |||
"dependencies": { | |||
"apollo-server-types": "^0.9.0 || ^3.0.0" |
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.
Is this a stray addition?
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.
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.
I had to add it to get it to work, but I'm not sure why.
graphql@15
It looks like that although OperationTypeNode existed as an exported type in graphql15, it did not exist as an enum, so the way I'm using it is incorrect in peer dependencies.
Fixes #1481