refactor: centralize GSI accessor helper (follow-up to #3505)#3511
Draft
Simone319 wants to merge 1 commit into
Draft
refactor: centralize GSI accessor helper (follow-up to #3505)#3511Simone319 wants to merge 1 commit into
Simone319 wants to merge 1 commit into
Conversation
Dedupe the aws-cdk-lib 2.252/2.260 _globalSecondaryIndexes fallback across 3 packages; no behavior change [follow-up to #3505] Extracts the dual-path accessor `table['_globalSecondaryIndexes'] ?? table.globalSecondaryIndexes` into a single exported helper `getGlobalSecondaryIndexes` in @aws-amplify/graphql-transformer-core and points all three call sites at it (relational-transformer resolvers, transformer-core schema-utils getKeySchema, conversation-transformer resolver generator). Identical dual-path semantics preserved.
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
1 participant
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.
What
Centralizes the dual-path DynamoDB GSI accessor introduced in #3505 into a single shared helper and points all three consumers at it.
aws-cdk-lib2.252/2.260 renamed the DynamoDB L2TableinternalglobalSecondaryIndexesarray to_globalSecondaryIndexes(now anArrayBox). #3505 landed the fallback expressiontable['_globalSecondaryIndexes'] ?? table.globalSecondaryIndexes, but the identical accessor ended up duplicated in 3 packages.New export in
@aws-amplify/graphql-transformer-core(src/utils/schema-utils.ts):Why
Per sarayev's review suggestion (idea B) on #3505: dedupe the 3× accessor so a future CDK rename is a one-line fix in one place instead of a hunt across packages.
Updated call sites (all 3)
packages/amplify-graphql-relational-transformer/src/resolvers.ts— removed the localgetGlobalSecondaryIndexesin favor of the shared one;.some(gsi => gsi.indexName === …)logic unchanged.packages/amplify-graphql-transformer-core/src/utils/schema-utils.ts—getKeySchemanow calls the shared helper for the GSI array (LSI accessor left inline, out of scope).packages/amplify-graphql-conversation-transformer/.../conversation-resolver-generator.ts— inline accessor replaced with the shared helper;.find(…).keySchemalogic unchanged.All three already depend on
@aws-amplify/graphql-transformer-core, so no new/circular dependency is introduced.No behavior change
This is a pure refactor + dedupe. The dual-path semantics are byte-for-byte identical — notably it does not add
?? []or otherwise harden the accessor (that hardening is a separate decision). Return type kept asanyto match exactly what the call sites rely on (.find(...).keySchemawithout a null-guard).It also does not remove the private-field dependency: a truly private-free accessor would require a managed-vs-standard-table branch, which is deliberately out of scope here.
Testing
graphql-transformer-coreunit tests: 92 passedgraphql-relational-transformerunit tests: 194 passedgraphql-conversation-transformerunit tests: 25 passedtsc), prettier applied,API.mdregenerated (only the new export added).Follow-up to #3505.