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

update to handle graphql v16_x breaking changes #495

Merged
merged 16 commits into from Jun 23, 2023

Conversation

bueche
Copy link
Contributor

@bueche bueche commented Jun 1, 2023

Description

The purpose of this PR is to upgrade join-monster to support graphql@16_6_0. There were a number of breaking changes introduced in graphql version 16 that had a major impact (mostly) to the tests and to some of the code in join-monster. This PR fixes these changes so the tests pass. I can't guarantee that other breaking changes were not caught by the tests.
Some of the breaking fixes of graphql v16:

  1. Remove support for positional args in graphql/execute/subscribe func graphql/graphql-js#2904. This was, by far, the largest change as it broke most tests due to their reliance on lodash partial. I just removed that dependency and fixed the parameters so that they would align with the v16 approach. So for example:
import { partial } from 'lodash'
    const run = partial(graphql, schemaBasic)
    const { data, errors } = await run(query) <--- old approach

was changed to:

      const { data, errors } = await graphql({schema, source}) <--- new approach
  1. Fragment tests failed due to a change in structure. I couldn't find the PR in graphql that impacted this one but the changes were mostly the main join-master index.js. Without these changes a number of the relay tests failed with the following error: def.args is not iterable and some code in graphql was added to iterate through this structure which in most cases was an array (only the failed cases were not an array). So I changed this definition to be an array (see src/index.js change.

  2. Other fragment tests failed with Support for returning GraphQLObjectType from resolveType was removed in graphql-js@16.0.0 please return type name instead and this was fixed also in src/index.js to add name to type.

  3. An error occurred: TypeError: Class constructor GraphQLNonNull cannot be invoked without 'new' that was fixed as well.

Testing

To test this change it is necessary (still) to use node v14 (nvm use 14).

It is important to ensure that graphql@16_6_0 is installed.

I ran the following suites:

  • npm run testsqlite3
  • npm run testpg
  • npm run testpg-paging (postgres)
  • npm run testmysql and
  • npm run testmysql-paging (mysql)

all passed, except for the offset cursor tests in mysql which were throwing an error This type of pagination not supported on this dialect which is a join-monster error and I believe expected.

  • This change adds test coverage for new/changed/fixed functionality (note: I did not add any new tests as the code and tests were just updated to support the breaking changes of graphql v16)

Checklist

  • I have added documentation for new/changed functionality in this PR via comments and by updating the change log

@bueche bueche changed the title initial commit to fix issues with graphsql v16_x breaking changes update to handle graphsql v16_x breaking changes Jun 1, 2023
@bueche bueche changed the title update to handle graphsql v16_x breaking changes update to handle graphql v16_x breaking changes Jun 1, 2023
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@bueche bueche merged commit 0c05d20 into join-monster:master Jun 23, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants