feat: @cacheTag composition for root query fields only - #3131
Open
SkArchon wants to merge 4 commits into
Open
Conversation
Contributor
WalkthroughChangesCache tag directive
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3131 +/- ##
==========================================
- Coverage 48.87% 45.73% -3.14%
==========================================
Files 1130 868 -262
Lines 157574 126773 -30801
Branches 10879 10937 +58
==========================================
- Hits 77009 57982 -19027
+ Misses 78696 68446 -10250
+ Partials 1869 345 -1524
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
composition/src/errors/errors.ts (1)
2182-2184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWording is misleading for the unclosed-brace case.
This message fires both for an extra/stray brace and for an unclosed placeholder (e.g.
"products-{$args.searchKey", per the "unclosed placeholder" test). "defines an extra curly brace" doesn't accurately describe a missing closing brace scenario.✏️ Suggested wording fix
export function unbalancedCacheTagFormatErrorMessage(format: string): string { - return `The "format" argument defines an extra curly brace; received "${format}".`; + return `The "format" argument contains an unbalanced curly brace; received "${format}".`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition/src/errors/errors.ts` around lines 2182 - 2184, Update unbalancedCacheTagFormatErrorMessage to use wording that accurately covers both stray/extra braces and unclosed placeholders, without asserting that the format specifically contains an extra curly brace.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@composition/src/v1/constants/directive-definitions.ts`:
- Around line 876-895: Run the repository’s Prettier formatter on
composition/src/v1/constants/directive-definitions.ts lines 876-895, including
the CACHE_TAG_DEFINITION block, and
composition/tests/v1/directives/cache-tag.test.ts lines 87-104; apply only the
formatting changes required for the composition lint check.
---
Nitpick comments:
In `@composition/src/errors/errors.ts`:
- Around line 2182-2184: Update unbalancedCacheTagFormatErrorMessage to use
wording that accurately covers both stray/extra braces and unclosed
placeholders, without asserting that the format specifically contains an extra
curly brace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5273bd8c-d306-40bd-a0bc-1afa2e870c50
📒 Files selected for processing (12)
composition/src/directive-definition-data/directive-definition-data.tscomposition/src/errors/errors.tscomposition/src/errors/types/params.tscomposition/src/router-configuration/types.tscomposition/src/router-configuration/utils.tscomposition/src/utils/string-constants.tscomposition/src/v1/constants/constants.tscomposition/src/v1/constants/directive-definitions.tscomposition/src/v1/normalization/normalization-factory.tscomposition/src/v1/normalization/types/types.tscomposition/src/v1/normalization/utils.tscomposition/tests/v1/directives/cache-tag.test.ts
SkArchon
marked this pull request as ready for review
July 30, 2026 18:25
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
composition/src/v1/normalization/normalization-factory.ts (1)
4526-4538: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider documenting the unresolved-type fallback in
isValidCacheTagLeaf.Returning
truewhennamedTypeDatais undefined is consistent with this file's existing pattern of deferring "undefined type" errors to a later global check (seevalidateArgumentsand the mainnormalize()loop, which use// undefined types are handled elsewherecomments). A short inline comment here would make that intent explicit for future readers, since without it the fallback could look like an oversight.📝 Optional clarifying comment
isValidCacheTagLeaf({ namedTypeName, type }: FieldData | InputValueData): boolean { if (isTypeNodeListType(type)) { return false; } if (BASE_SCALARS.has(namedTypeName)) { return true; } const namedTypeData = this.parentDefinitionDataByTypeName.get(namedTypeName); if (!namedTypeData) { + // Undefined types are handled elsewhere (a separate undefined-type error is raised globally). return true; } return namedTypeData.kind === Kind.SCALAR_TYPE_DEFINITION || namedTypeData.kind === Kind.ENUM_TYPE_DEFINITION; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition/src/v1/normalization/normalization-factory.ts` around lines 4526 - 4538, Add a brief inline comment in isValidCacheTagLeaf immediately before the !namedTypeData fallback, documenting that unresolved or undefined types are intentionally accepted here and validated later by the global checks. Keep the existing return behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@composition/src/v1/normalization/normalization-factory.ts`:
- Around line 4526-4538: Add a brief inline comment in isValidCacheTagLeaf
immediately before the !namedTypeData fallback, documenting that unresolved or
undefined types are intentionally accepted here and validated later by the
global checks. Keep the existing return behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea1d8ac1-bf7c-4002-b228-c3dce0176789
📒 Files selected for processing (13)
composition/src/directive-definition-data/directive-definition-data.tscomposition/src/errors/errors.tscomposition/src/errors/types/params.tscomposition/src/router-configuration/types.tscomposition/src/router-configuration/utils.tscomposition/src/utils/string-constants.tscomposition/src/v1/constants/constants.tscomposition/src/v1/constants/directive-definitions.tscomposition/src/v1/normalization/normalization-factory.tscomposition/src/v1/normalization/types/types.tscomposition/src/v1/normalization/utils.tscomposition/tests/v1/directives/cache-tag.test.tscomposition/tests/v1/normalization-utils.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- composition/src/router-configuration/utils.ts
- composition/src/v1/normalization/types/types.ts
- composition/tests/v1/normalization-utils.test.ts
- composition/src/directive-definition-data/directive-definition-data.ts
- composition/src/utils/string-constants.ts
- composition/src/v1/constants/directive-definitions.ts
- composition/src/router-configuration/types.ts
- composition/src/errors/errors.ts
- composition/src/v1/constants/constants.ts
- composition/tests/v1/directives/cache-tag.test.ts
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.
This PR introduces
@cacheTag, with the following limitationsSummary by CodeRabbit
New Features
@cacheTag(format: String!)directive on query fields.formatstrings with static text and$argsplaceholders, including nested input-value paths.Validation
Tests
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.