Repository navigation
[visitor-plugin-common] fix: keep local fragment definition over its external copy (#10932) - #11048
Open
akashchamp wants to merge 1 commit into
Open
akashchamp wants to merge 1 commit into
akashchamp wants to merge 1 commit into
Conversation
…external copy (dotansimha#10932) ClientSideBaseVisitor dedupes fragments by name into a Map, so when the same fragment is passed both as a local definition and as an external fragment (near-operation-file does this for fragments generated into the same file), the external copy won and the fragment document was no longer generated, while operations still interpolated it. Keep the local definition in that case, as before 2.13.8.
🦋 Changeset detectedLatest commit: 35b92cc 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 |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related #10932
Since #8757,
ClientSideBaseVisitordedupes fragments by name withnew Map(...), so the last entry wins.near-operation-filepasses a fragment that is generated into the same output file both as a local definition and as an external fragment, so the external copy replaced the local one andget fragments()skipped it. The operation still interpolates${ThingFieldsFragmentDoc}, so the generated file doesn't compile.This keeps the local definition when the same fragment name also comes in as external (in either order), which is what happened before 2.13.8 with the array plus
dependency-graph. Fragments from other files are unchanged: they are still only imported.Type of change
How Has This Been Tested?
visitor-plugin-common/tests/client-side-base-visitor.spec.tsthat passes the same fragment as local and external. It fails onmaster(expected '' to contain 'export const UserFieldsFragmentDoc') and passes with the fix.@graphql-codegen/cli7.4.5,near-operation-file-preset5.2.2,typescript-operations6.1.10 andtyped-document-node7.1.1 withdocumentMode: 'graphQLTag'. With the publishedvisitor-plugin-common7.2.9 the generatedThing.generated.tshas noThingFieldsFragmentDocandtscfails withTS2304: Cannot find name 'ThingFieldsFragmentDoc'. With this branch's build ofvisitor-plugin-commonswapped in, the doc is emitted andtsc --noEmit --strictpasses. I also added a fragment in a separate file spread from another file's query: it is still imported from./Other.generated, not redeclared.pnpm vitest run,tsc --noEmit,eslintandprettier --checkon the changed files. The only vitest failure (besides a watcher test that times out under load and passes on its own) is theimport-typessnapshot, which depends on the clone directory name and fails onmastertoo.Test Environment:
@graphql-codegen/visitor-plugin-common: 7.2.9 (master)