refactor(types): name the trait value shape - #8366
Conversation
featureStateToValue asserted an inline `{ value_type?: ... }` to read the
type key core traits use, so the key was tied to nothing and a rename was
silent. TraitValue names the shape, `in` narrows the union instead of
asserting, and annotating Utils.valueToTrait as the producer means both
ends of the contract share one type.
float_value and 'float' are included because traits carry both
(api/environments/identities/traits/models.py).
Closes #8365
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe change adds the exported Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change preserves behavior while naming the trait-value shape; the remaining inline-union style issue is localized and non-blocking, so the PR is merge-ready after normal checks. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
front-end
talissoncosta
changed the title
Docker builds report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 692c445b-67f1-4f45-97da-4e79702e05f4
📒 Files selected for processing (4)
frontend/common/types/responses.tsfrontend/common/utils/__tests__/featureStateToValue.test.tsfrontend/common/utils/featureStateToValue.tsfrontend/common/utils/utils.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| float_value?: number | null | ||
| integer_value: number | null | ||
| string_value: string | null | ||
| value_type: 'int' | 'unicode' | 'bool' | 'float' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract the inline value_type union into a named type.
Line 605 adds an inline union type. The frontend TypeScript guideline requires union types to use named aliases. Define a named alias such as TraitValueType and use it for TraitValue.value_type. Reuse the alias for FeatureStateValue.type if both fields share the same contract.
As per coding guidelines: frontend/**/*.{ts,tsx} requires inline union types to be extracted into named types.
Proposed refactor
+export type TraitValueType = 'int' | 'unicode' | 'bool' | 'float'
+
export type TraitValue = {
boolean_value: boolean | null
float_value?: number | null
integer_value: number | null
string_value: string | null
- value_type: 'int' | 'unicode' | 'bool' | 'float'
+ value_type: TraitValueType
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| value_type: 'int' | 'unicode' | 'bool' | 'float' | |
| export type TraitValueType = 'int' | 'unicode' | 'bool' | 'float' | |
| export type TraitValue = { | |
| boolean_value: boolean | null | |
| float_value?: number | null | |
| integer_value: number | null | |
| string_value: string | null | |
| value_type: TraitValueType | |
| } |
Source: Coding guidelines
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19755 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #19755 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19755 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19755 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
2 participants
docs/if required so people know about the feature.Changes
Closes #8365, follow-up to Wadii's comment on #8344.
featureStateToValuecast an inline{ value_type?: ... }to read the type key core traits use, so the key was tied to nothing and a rename was silent.TraitValuenames that shape andinnarrows the union instead.Utils.valueToTraitgets the same type as its return, which is the half that actually catches renames.How did you test this code?
tscerror count unchanged against mainvalue_typenow fails in the consumer, the producer and the tests. Before, nowhere