feat: rename declareStatic → derive and <Static> → <Derive>#1060
feat: rename declareStatic → derive and <Static> → <Derive>#1060moss-bryophyta wants to merge 9 commits intogeneraltranslation:mainfrom
Conversation
07ca12c to
cdf6dfc
Compare
|
@greptile-ai re-review |
1 similar comment
|
@greptile-ai re-review |
| decodeMsg, | ||
| decodeOptions, | ||
| declareStatic, | ||
| derive, |
There was a problem hiding this comment.
you have missed the required changes in the SWC plugin inside of packages/next, it skips hash calculation when it comes across a static/declare static. of course still needs to be backwards compatible.
|
@greptileai review this PR again please. You need to check for backwards compatibility. What that means is literally literally every single mention of Static or declareStatic needs to be accompanied by the new Derive component and derive functions. You need to make sure that EVERY SINGLE ONE is accompanied. Regardless of language, package, etc.. It is very important that we do not introduce backward compatibility changes. |
…C plugin - Rename DECLARE_DERIVE_FUNCTION to DERIVE_FUNCTION in cli constants - Remove DECLARE_DERIVE_FUNCTION_NAME alias from linter constants - Add 'Derive' to variable component recognition in SWC plugin - Add 'derive' to declare static function recognition in SWC plugin - Map 'Derive' to VariableType::Static in SWC utilities - Update error messages to mention derive() alongside declareStatic()
| export function Static<T extends React.ReactNode>({ | ||
| children, | ||
| }: { | ||
| children: T; | ||
| }): T { | ||
| return children; | ||
| } |
There was a problem hiding this comment.
Deprecated alias has a duplicate implementation
Static is a copy-paste of Derive's implementation rather than a re-export alias. If Derive's implementation ever changes, Static would silently diverge. Delegating to the canonical function is safer and removes duplicated code:
| export function Static<T extends React.ReactNode>({ | |
| children, | |
| }: { | |
| children: T; | |
| }): T { | |
| return children; | |
| } | |
| export const Static = Derive; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-core/src/variables/Derive.tsx
Line: 57-63
Comment:
**Deprecated alias has a duplicate implementation**
`Static` is a copy-paste of `Derive`'s implementation rather than a re-export alias. If `Derive`'s implementation ever changes, `Static` would silently diverge. Delegating to the canonical function is safer and removes duplicated code:
```suggestion
export const Static = Derive;
```
How can I resolve this? If you propose a fix, please make it concise.
packages/core/src/derive/derive.ts
Outdated
| export function declareStatic< | ||
| T extends string | boolean | number | null | undefined, | ||
| >(content: T): T { | ||
| return content; | ||
| } |
There was a problem hiding this comment.
Deprecated alias has a duplicate implementation
declareStatic is independently implemented rather than delegating to derive. Both are trivial pass-throughs right now, but any future change to derive would need to be manually mirrored here. Using a re-export alias would make the deprecation relationship explicit and future-proof:
| export function declareStatic< | |
| T extends string | boolean | number | null | undefined, | |
| >(content: T): T { | |
| return content; | |
| } | |
| export const declareStatic = derive; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/derive/derive.ts
Line: 43-47
Comment:
**Deprecated alias has a duplicate implementation**
`declareStatic` is independently implemented rather than delegating to `derive`. Both are trivial pass-throughs right now, but any future change to `derive` would need to be manually mirrored here. Using a re-export alias would make the deprecation relationship explicit and future-proof:
```suggestion
export const declareStatic = derive;
```
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (3)
This error message (and several others in this file) still refers exclusively to Affected lines: 40, 57, 302, and 320. All four should be updated to mention both Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/compiler/src/transform/validation/validateTranslationFunctionCallback.ts
Line: 40
Comment:
**Stale `declareStatic` references in user-facing error messages**
This error message (and several others in this file) still refers exclusively to `declareStatic`, even though the rename introduces `derive()` as the canonical name. Users who call `derive()` and hit this error will see a confusing message pointing them to the old API name.
Affected lines: 40, 57, 302, and 320. All four should be updated to mention both `derive()` / `declareStatic()` (similar to how the `warnDeclareStaticNotWrappedSync` message in the CLI was updated in this PR).
```suggestion
'useGT_callback / getGT_callback must use a string literal or derive() (or declareStatic()) call as the first argument. Variable content is not allowed.'
```
How can I resolve this? If you propose a fix, please make it concise.
This error message is produced by a helper called Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/compiler/src/transform/validation/validateTranslationFunctionCallback.ts
Line: 302
Comment:
**Stale `declareStatic` reference in error message**
This error message is produced by a helper called `validateDeclareStaticExpression` which is invoked for both `declareStatic(...)` and `derive(...)` calls (see `validateDeclareStatic` above). A user calling `derive(42, "extra")` will see `'DeclareStatic must have one argument'`, which refers to the old name only.
```suggestion
errors.push('derive() / declareStatic() must have one argument');
```
How can I resolve this? If you propose a fix, please make it concise.
Same issue as line 302 — this message is shown when Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/compiler/src/transform/validation/validateTranslationFunctionCallback.ts
Line: 320
Comment:
**Stale `declareStatic` reference in error message**
Same issue as line 302 — this message is shown when `derive()` is called with a non-call argument, but only names `declareStatic`.
```suggestion
errors.push('derive() / declareStatic() must have a call expression as the argument');
```
How can I resolve this? If you propose a fix, please make it concise. |
|
@greptileai please review the rust implementation |
…ed aliases - Static component now delegates to Derive instead of duplicating impl - declareStatic function now delegates to derive instead of duplicating impl Addresses review feedback from Ernest.
Additional Comments (3)
The following error messages should be updated to mention
Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/compiler/src/transform/validation/validateTranslationFunctionCallback.ts
Line: 40
Comment:
User-facing error messages still reference `declareStatic` exclusively, even though `derive()` is now the canonical name. A user who correctly adopts `derive()` will see error messages pointing them to the old API, which is confusing.
The following error messages should be updated to mention `derive()` (optionally noting the deprecated alias):
- Lines 40 & 57: `"...must use a string literal or declareStatic call..."` → mention `derive()`
- Line 301: `"DeclareStatic must have one argument"` → `"derive() must have one argument"`
- Lines 310 & 319: `"DeclareStatic must have a call expression as the argument"` → `"derive() must have a call expression as the argument"`
```suggestion
'useGT_callback / getGT_callback must use a string literal or derive() call as the first argument. Variable content is not allowed.'
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/cli/src/react/jsx/utils/parseDerive.ts
Line: 45
Comment:
Several comments in this file still reference `declareStatic` instead of the canonical name `derive()`. Since the file was renamed and the canonical function is now `derive()`, comments should be updated for consistency:
- Line 45: `"...uses declareStatic"` → `"...uses derive"`
- Line 74: `"// We found declareStatic"` → `"// We found derive()"`
- Line 216: `"Given a CallExpression, if it is declareStatic(<call>)..."` → update to `derive`
- Line 252: `"// Must be declareStatic(...)"` → `"// Must be derive(...)"`
- Line 265: Invert wording now that `derive` is primary: `"// Only proceed if the name is 'derive' (or the deprecated 'declareStatic')"`
- Line 273: `"...so it's not declareStatic"` → `"...so it's not derive"`
```suggestion
* Checks if an expression is static or uses derive()
```
How can I resolve this? If you propose a fix, please make it concise.
Either:
Leaving this dead code risks lint warnings and confuses future maintainers about whether this feature is intentionally disabled or accidentally orphaned. Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/cli/src/react/jsx/utils/parseDerive.ts
Line: 779-785
Comment:
The `collectConditionalStringVariants()` function defined here (lines 779–785) is unreachable dead code. Its only callers are in the large commented-out block above (lines 315–338, specifically lines 326 and 336).
Either:
1. Uncomment and re-enable the conditional-expression handling (lines 315–338) and its callers, or
2. Remove both the function and the commented-out block to keep the file clean
Leaving this dead code risks lint warnings and confuses future maintainers about whether this feature is intentionally disabled or accidentally orphaned.
How can I resolve this? If you propose a fix, please make it concise. |
… of declareStatic - compiler error messages now mention derive() as the canonical name - parseDerive.ts comments updated for consistency - SWC state.rs comment updated to include Derive Addresses additional review feedback from Ernest.
|
moved to #1062 |
…` to `derive()` (#1062) ## Overview We want to rename `<Static>` to `<Derive>` and rename `declareStatic()` to `derive()`. ## Motivation We have tested a few options, and the word "derive" performs much better for human and model understanding ## Notes - We will not change `s` field in the `JsxChildren` data structure as we want to avoid unnecessary compatibility changes for external services. - based from #1060 <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR performs a broad rename across 70 files: the `<Static>` JSX component becomes `<Derive>`, and the `declareStatic()` function becomes `derive()`. Both old names are retained as `@deprecated` aliases, and the internal `'s'` wire-format field is intentionally left unchanged for external-service compatibility. **Key changes:** - `packages/core/src/static/` directory moved to `packages/core/src/derive/`, with `derive.ts` introducing the new `derive()` function and `declareStatic = derive` as a backward-compat alias. - `packages/react-core/src/variables/Static.tsx` deleted; `Derive.tsx` created with `Static = Derive` alias. - `MINIFY_CANONICAL_NAME_MAP` maps both `Derive` and `Static` to `'s'`; `metadata.staticId` field name is preserved — consistent with the stated backward-compatibility goal. - All public-facing exports across `gt-react`, `gt-next`, `gt-node`, and the SWC plugin updated to expose `derive` / `Derive` alongside (or instead of) the deprecated counterparts. - Linter helpers (`isDeriveComponent`, `isDeriveFunction`) updated to check both old and new names, preserving lint correctness for codebases still using `<Static>`/`declareStatic`. - One minor inconsistency: the private functions `validateDeclareStatic` and `validateDeclareStaticExpression` inside `validateTranslationFunctionCallback.ts` retain their old names even though their JSDoc and emitted error strings now reference `derive()`. <details open><summary><h3>Confidence Score: 5/5</h3></summary> - Safe to merge — purely additive rename with full backward-compatibility aliases and no logic changes. - The PR is a well-structured rename refactor across 70 files. Backward-compat aliases are correctly wired at every layer (core, react, next, node, compiler, linter, SWC plugin). Wire format is intentionally unchanged. No logic is altered. The only finding is a minor internal naming inconsistency in two private functions inside the compiler validation module, which carries no runtime or API impact. - packages/compiler/src/transform/validation/validateTranslationFunctionCallback.ts — private helper names are inconsistent with updated JSDoc/error messages. </details> <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | packages/core/src/derive/derive.ts | New `derive()` implementation with backward-compatible `declareStatic` alias; clean and correct. | | packages/react-core/src/variables/Derive.tsx | New `<Derive>` component with `Static = Derive` backward-compat alias; JSDoc `@param`/`@returns` descriptions still reference "The result of the function invocation" rather than describing the component's passthrough semantics (flagged in prior review thread). | | packages/compiler/src/transform/validation/validateTranslationFunctionCallback.ts | Error messages and JSDoc updated to reference `derive()`, but the private functions `validateDeclareStatic` and `validateDeclareStaticExpression` retain their old names, creating an internal naming inconsistency. | | packages/compiler/src/utils/constants/gt/helpers.ts | Correctly renames `isStaticComponent` to `isDeriveComponent` (checking both `Static` and `Derive`), preserves the old name as a deprecated alias; both `Derive` and `Static` share the `'static'` default variable name, which is intentional. | | packages/compiler/src/utils/constants/gt/constants.ts | Adds `Derive` to `GT_COMPONENT_TYPES` enum and maps it to `'s'` in `MINIFY_CANONICAL_NAME_MAP`, consistent with the PR's intent to preserve the wire format. | | packages/next/src/index.types.ts | Adds `Derive` stub, moves `declareStatic` to a dedicated deprecated re-export while promoting `derive` in the main export block; correctly structured for backward compatibility. | | packages/react-core-linter/src/utils/isGTFunction.ts | Correctly renames `isStaticComponent`→`isDeriveComponent` and `isDeclareStaticFunction`→`isDeriveFunction`, each checking both old and new names for backward compatibility. | | packages/next/swc-plugin/src/visitor/analysis.rs | Correctly adds `"Derive"` to variable-component and declare-static checks in Rust SWC plugin; test updated accordingly. | | packages/cli/src/console/index.ts | Error helpers renamed and messages updated to reference `<Derive>` and `derive()`; also fixes a pre-existing typo ("does have" → "does not have", "statment" → "statement") in `warnMissingReturnSync`. | | packages/cli/src/extraction/postProcess.ts | Renames `linkStaticUpdates` → `linkDeriveUpdates` and internal variables; correctly retains `metadata.staticId` field name per the PR's wire-format stability note. | </details> </details> <details><summary><h3>Flowchart</h3></summary> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD subgraph Public API ["Public API (new → deprecated alias)"] D1["derive()"] -->|"alias"| D2["declareStatic() @deprecated"] C1["<Derive>"] -->|"alias"| C2["<Static> @deprecated"] end subgraph Core ["generaltranslation / core"] D1 --> CoreDerive["core/src/derive/derive.ts"] C1 --> ReactDerive["react-core/src/variables/Derive.tsx"] end subgraph Compiler ["Compiler / CLI"] CoreDerive --> CompConst["GT_OTHER_FUNCTIONS.derive\n+ GT_OTHER_FUNCTIONS.declareStatic"] ReactDerive --> CompHelper["isDeriveComponent()\nchecks 'Derive' | 'Static'"] CompConst --> Validate["validateDeclareStatic()\n(private, checks both names)"] end subgraph Linter ["react-core-linter"] C1 --> LintDerive["isDeriveComponent()"] D1 --> LintFn["isDeriveFunction()"] LintDerive -->|"checks"| Both1["'Derive' | 'Static'"] LintFn -->|"checks"| Both2["'derive' | 'declareStatic'"] end subgraph SWC ["next/swc-plugin (Rust)"] C1 --> RST["is_variable_component_name()\n'Derive' | 'Static' | …"] D1 --> RSD["is_declare_static_name()\n'derive' | 'declareStatic'"] end subgraph WireFormat ["Wire format (unchanged)"] C1 -->|"minified as"| WF["'s' (staticId field preserved)"] C2 -->|"minified as"| WF end ``` </details> <details><summary>Prompt To Fix All With AI</summary> `````markdown This is a comment left during a code review. Path: packages/compiler/src/transform/validation/validateTranslationFunctionCallback.ts Line: 208-215 Comment: **Stale private function names after rename** The two private functions `validateDeclareStatic` (line ~208) and `validateDeclareStaticExpression` (line ~288) were not renamed alongside the public API and their JSDoc/error messages. Their JSDoc comments now reference `derive()` but the function identifiers still say `DeclareStatic`, which makes the file inconsistent to maintain: ``` // JSDoc says: "Validates if an expression uses the derive() function correctly" function validateDeclareStatic(...) // ← name still says DeclareStatic ``` ``` // JSDoc says: "Example: derive(getName())" function validateDeclareStaticExpression(...) // ← name still says DeclareStatic ``` Consider renaming them to `validateDerive` / `validateDeriveExpression` to match the updated doc comments and error strings. The same stale reference appears in the test comment at line 114 of `validateTranslationFunctionCallback.test.ts`: ``` // When an identifier is passed, validateDeclareStatic adds errors ``` How can I resolve this? If you propose a fix, please make it concise. ````` </details> <sub>Last reviewed commit: 89166b4</sub> <!-- /greptile_comment --> --------- Co-authored-by: moss <moss@generaltranslation.com> Co-authored-by: moss-bryophyta <261561981+moss-bryophyta@users.noreply.github.com>
Summary
Renames the canonical implementations to use the new names:
declareStatic()→derive()(function)<Static>→<Derive>(React component)Approach
derive/DerivedeclareStaticandStaticre-export the new namesChanges
Core source (renamed):
packages/core/src/static/declareStatic.ts→derive.ts— function is nowderive(), withdeclareStaticas deprecated aliaspackages/react-core/src/variables/Static.tsx→Derive.tsx— component is now<Derive>, withStaticas deprecated aliaspackages/core/src/static/index.ts— updated re-exportPackage exports (all export both new + old names):
react-core(index.ts, internal.ts)react(client.ts, index.ts, internal.ts)next(index.client.ts, index.server.ts, index.types.ts, server.ts)node(translation-functions/index.ts)i18n(translation-functions/static/index.ts)CLI/Compiler/Linter (recognize both names):
DERIVE_FUNCTION/DERIVE_COMPONENTparseDeclareStatic.tsrecognizesderivealongsidedeclareStaticDerive/deriveentriesisGTFunction.tsrecognize both namesGreptile Summary
This PR renames the canonical
declareStatic()function toderive()and the<Static>component to<Derive>across the entire monorepo (55 files). The old names are preserved as@deprecatedaliases in all public package exports, the CLI, compiler, linter, and the Rust SWC plugin. The approach is sound and backward-compatible.Key changes:
packages/core/src/static/directory moved toderive/, withderive.tsas the new source of truth exporting bothderiveand a deprecateddeclareStaticpackages/react-core/src/variables/Derive.tsxreplacesStatic.tsx, exporting bothDeriveand a deprecatedStaticgt-react,gt-next,gt-node,gt-i18n) re-export both names with proper deprecation markersparseDeclareStatic.tsrenamed toparseDerive.tsand updated to recognize bothderiveanddeclareStaticimport namesGT_OTHER_FUNCTIONSgains aderiveentry;GT_COMPONENT_TYPESgainsDeriveIssues to address:
validateTranslationFunctionCallback.tsstill exclusively referencedeclareStatic, which will confuse users who adopt the newderive()nameparseDerive.tsstill refer todeclareStaticinstead ofderivecollectConditionalStringVariantsinparseDerive.tsis unreachable dead code — its only callers are in a commented-out blockConfidence Score: 3/5
derive()adopters and stale documentation.declareStatic, which will confuse users correctly adoptingderive(), (2) several comments inparseDerive.tsstill refer to the old name instead of the canonical name, making the code harder to follow for future maintainers, and (3) unreachable dead code (collectConditionalStringVariants) should be removed or the commented-out conditional handling should be re-enabled. These are fixable and don't introduce regressions, but addressing them would improve the quality of the PR.Last reviewed commit: f11ffb2