[graphql-fixtures] Graphql fixtures/seed collision fix

[Flufd]

Description

Fixes #2311

Uses a quick hash of the strings that make up a graphQL object's keypath to generate a random seed for that object, instead of a sum of all of the characters values.

I think this is a breaking change, only if people are asserting on the random values that are already generated.

TODO: changelog, once I know if this is considered breaking.

Type of change

• Patch: Bug (non-breaking change which fixes an issue)
• Minor: New feature (non-breaking change which adds functionality)
• Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

[BPScott]

Looking good! I've just added a commit to remove the need for an as any cast in the test.

I think this is a minor change. End users shouldn't be relying on the randomly returned filler data. If there's some data that they want to fix then they should specify it.

You can cut a beta release and test this in web by commenting /snapit on the PR.

Add a changeset (yarn add changeset and follow instructions). and I think we're good to go. See https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md for more info.

[BPScott]

/snapit

[github-actions]

🫰✨ Thanks @BPScott! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/graphql-persisted@0.0.0-snapshot-20220622233020

yarn add @shopify/graphql-testing@0.0.0-snapshot-20220622233020

yarn add @shopify/koa-performance@0.0.0-snapshot-20220622233020

yarn add @shopify/koa-shopify-webhooks@0.0.0-snapshot-20220622233020

yarn add @shopify/network@0.0.0-snapshot-20220622233020

yarn add @shopify/react-cookie@0.0.0-snapshot-20220622233020

yarn add @shopify/react-graphql@0.0.0-snapshot-20220622233020

yarn add @shopify/react-graphql-universal-provider@0.0.0-snapshot-20220622233020

yarn add @shopify/react-network@0.0.0-snapshot-20220622233020

yarn add @shopify/react-router@0.0.0-snapshot-20220622233020

yarn add @shopify/react-server@0.0.0-snapshot-20220622233020

yarn add @shopify/sewing-kit-koa@0.0.0-snapshot-20220622233020

[BPScott]

Give that version of graphql-fixtures a go in web and check if the changing of the seed generation has a painful effect on tests.

[Flufd]

thanks @BPScott. Doesn't look like graphql-fixtures was cut in that release. I had a quick look why that might be, but couldn't figure it out.

[BPScott]

ah, duh, I forgot that we need a changeset in this branch that mentions graphql-fixtures before snapit shall pick up that the package needs to be released

(new toy, still internalising how it works)

[BPScott]

/snapit

[github-actions]

🫰✨ Thanks @BPScott! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add graphql-config-utilities@0.0.0-snapshot-20220623162708

yarn add graphql-fixtures@0.0.0-snapshot-20220623162708

yarn add @shopify/graphql-persisted@0.0.0-snapshot-20220623162708

yarn add @shopify/graphql-testing@0.0.0-snapshot-20220623162708

yarn add graphql-typescript-definitions@0.0.0-snapshot-20220623162708

yarn add graphql-validate-fixtures@0.0.0-snapshot-20220623162708

yarn add @shopify/koa-performance@0.0.0-snapshot-20220623162708

yarn add @shopify/koa-shopify-webhooks@0.0.0-snapshot-20220623162708

yarn add @shopify/network@0.0.0-snapshot-20220623162708

yarn add @shopify/react-cookie@0.0.0-snapshot-20220623162708

yarn add @shopify/react-graphql@0.0.0-snapshot-20220623162708

yarn add @shopify/react-graphql-universal-provider@0.0.0-snapshot-20220623162708

yarn add @shopify/react-network@0.0.0-snapshot-20220623162708

yarn add @shopify/react-router@0.0.0-snapshot-20220623162708

yarn add @shopify/react-server@0.0.0-snapshot-20220623162708

yarn add @shopify/sewing-kit-koa@0.0.0-snapshot-20220623162708

[BPScott]

yarn add graphql-fixtures@0.0.0-snapshot-20220623162708

there we go

[BPScott]

Good news: I think the code change is good. Improving the seed generation so it results in fewer trivial collisions (e.g. a[0].b[1] collides with a[1].b[0] and a[12] collides with a[21]) is a good thing.

Bad news: I suspected that consumers would rely on this buggy behaviour in some places. However it is worse than I thought - introducing this change in web results in 370ish test failures. I am pretty confident that this is web doing the wrong thing here, and that it is web's tests that need to be updated. However the sheer number of tests suggests that relying on this buggy behaviour is easier than I anticipated, and thus a release of graphql-fixtures containing change will require more work to merge cleanly. With that in mind, I think the least surprising thing is to claim this is a major version bump and write something more detailed in the changeset to describe why this would cause test failures.

Worse news: We are working on apollo3 in quilt and web and I don't want the release of this change to block the testing of that work. Thus I do not want to release this change until a version of graphql-fixtures containing the change can be cleanly used in web without test failures. That means you will need to go fix all the tests in web to not rely on this buggy behaviour, before we merge this PR.

Proposed Plan

• The CI for https://github.com/Shopify/web/pull/67518 shows all the test failures in web as a result of this update. Apply fixes for these tests, and backport them to the main branch (as it is possible to write fixed tests that work before and after this change).
• Once we get to a point where a PR containing just a graphql-testing bump passes CI, then we we can merge and release this change as a major version.
• Update web to use that new major version.

Are you able to corral a team to make these fixes in web?

[BPScott]

Marking this as blocked, till we update web to not rely on this buggy behaviour that we're removing.

We'll also want to update this to be a major change in the changeset