feat(extractor): resolve const namespace identifiers and NS.KEY member access#202
Open
bdshadow wants to merge 7 commits into
Open
feat(extractor): resolve const namespace identifiers and NS.KEY member access#202bdshadow wants to merge 7 commits into
bdshadow wants to merge 7 commits into
Conversation
When useTranslate(NS), <T ns={NS}>, or t('key', { ns: NS }) refers to
a top-level `const NS = '...'` (optionally annotated as const), the
extractor now substitutes the literal value instead of emitting
W_DYNAMIC_NAMESPACE. Lets users keep namespaces in a shared constants
file without forcing them to inline string literals everywhere.
Builds on top of the bare-identifier resolution: extractConstants now
also flattens `const NS = { K: 'literal' } as const` (and the plain
non-`as const` form) into `NS.K -> 'literal'` entries, and
getValue consumes `variable + dot + variable` sequences to look them
up.
Also maps the TextMate scopes used for member access in argument
position (`variable.other.constant.object.ts`,
`variable.other.constant.property.ts`, `variable.other.property.ts`)
to the `variable` customType so they reach the parser at all.
Cross-file imports of constants files still warn — Path A is
same-file only.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR adds top-level TypeScript const declaration extraction and resolution. A token-stream state machine extracts module-scope ChangesTop-level constant namespace resolution
Sequence Diagram(s)sequenceDiagram
participant CLI as Parser.parse
participant Tokens as FilteredTokens
participant Extract as extractConstants
participant Context as context.constants
participant getValue as getValue
CLI->>Tokens: provide filtered token array
CLI->>Extract: extractConstants(Tokens)
Extract->>Context: populate Map("NAME" / "NS.KEY" -> literal)
CLI->>getValue: resolve variable/member tokens during parse
getValue->>Context: lookup "NAME" or "NAME.PROP"
alt found
Context-->>getValue: literal -> return primitive
else not found
Context-->>getValue: undefined -> return expr placeholder
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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: CHILL
Plan: Pro
Run ID: 34965941-3241-4bf0-bf2e-cb2cc2dfca4b
📒 Files selected for processing (8)
src/extractor/extractor.tssrc/extractor/parser/extractConstants.tssrc/extractor/parser/generalMapper.tssrc/extractor/parser/parser.tssrc/extractor/parser/tree/getValue.tssrc/extractor/parser/types.tstest/unit/extractor/react/tComponent.test.tstest/unit/extractor/react/useTranslate.test.ts
This was referenced May 21, 2026
Walks the already-merged TextMate token stream instead of regex-scanning the raw source. Strings, comments and template literals are no longer ambiguous (TextMate has classified them upstream), and a funcDepth counter ensures we only capture declarations at module top level. This addresses the scope-shadowing concern raised in PR review: a `const NS` declared inside a function body no longer leaks into the top-level constants map. Added a regression test pinning the behaviour. No behavioural change for the documented happy paths; existing 249 react tests plus the new case pass.
Each sub-state of the walker now has its own function: - stepInFunctionBody (skip-and-track block depth) - stepInObjectBody (object literal property capture) - stepAtTopLevel (the main const-declaration FSM) - finalizeObjectCapture / resetCapture / createContext extractConstants() is now a small dispatch loop. No behaviour change; all existing tests pass.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/extractor/parser/extractConstants.ts`:
- Around line 89-95: The current loop in the non-abandoned branch uses
ctx.result.has(composed) to avoid overwriting existing entries, which preserves
the first-write instead of JavaScript's last-write-wins semantics; change the
behavior in the block that iterates ctx.captureProps so that for each { key,
value } you always assign into ctx.result (e.g., call ctx.result.set(composed,
value) unconditionally) so later properties on the same captured object
(identified by ctx.captureName + '.' + key) overwrite earlier ones; keep the
check for ctx.abandonedObject as-is so this change only affects the
non-abandoned path.
- Around line 166-170: The code incorrectly treats a list.begin token like a
block by setting ctx.funcDepth = 1 (in the branch handling "c === 'block.begin'
|| c === 'list.begin'"), which relies on stepInFunctionBody() to consume the
matching end token but stepInFunctionBody() never consumes list.end; change the
logic so that list.begin is handled separately from block.begin: do not set
ctx.funcDepth when c === 'list.begin'—instead skip the array/destructure pattern
until you see the corresponding 'list.end' token (or use a short local skip
loop) and then resume with ctx.state = 'Idle'; keep the existing funcDepth
behavior only for block.begin so top-level array destructures (e.g. const [x] =
...) do not leave ctx.funcDepth > 0 and block extraction continues correctly.
🪄 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: 935005c8-8f92-4862-a3dd-b7f94fb1b6ba
📒 Files selected for processing (1)
src/extractor/parser/extractConstants.ts
The finalize loop was guarding with `!result.has(composed)`, which
preserved the first-encountered value for duplicate keys within a
single `const NS = { ... }` literal. JS object-literal semantics are
last-write-wins, so the extractor's output diverged from what the
runtime actually sees. Always assign and let later properties win.
The cross-declaration guard in stepAtTopLevel (`const X = '...'`
followed by another `const X = '...'`) is unrelated and stays — a
re-declaration is a TS error anyway, and first-write is safer.
The AfterConst branch reused funcDepth for array-destructure
patterns (`const [a, b] = ...`), but stepInFunctionBody only
consumes block.begin/end. The matching list.end was therefore
ignored, leaving funcDepth permanently > 0 and silently skipping
every subsequent top-level const declaration.
Track list-bracket depth in its own listDepth counter with a
matching stepInListSkip dispatcher. Object destructure
(`const { x } = ...`) keeps the existing funcDepth path.
Regression test added.
The three `Object.keys(stored).toEqual([...])` assertions in push.p3 rely on the order in which the Tolgee API returns keys, which is not guaranteed and varies between server versions. Compare as a set by sorting both sides; matches the order-agnostic pattern already used in sync.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.
Problem
The extractor warns
W_DYNAMIC_NAMESPACEwhenever a namespace isreferenced through a constant instead of a literal:
This is exactly the shape customers reach for to avoid namespace typos
(a typed constants object on top of which TypeScript catches typos).
Today, choosing safety means losing extraction. The only workarounds
are bare literals everywhere or
@tolgee-keymagic comments at everycall site.
Solution
A small per-file constant pre-pass runs before parsing and builds a
flat
Map<string, string>:const NAME = 'literal' (as const)?→NAME → 'literal'const NS = { K1: 'l1', K2: 'l2' } as const→NS.K1 → 'l1',NS.K2 → 'l2'as constvariant too. Nestedobjects are intentionally skipped (existing dynamic warning still
fires).
The map is threaded through
ParserContext.constants.getValue.tsresolves bare
variabletokens via the map, and a one-step lookaheadcatches the
variable + dot + variable(member-access) pattern.A few additional TextMate scopes (
variable.other.constant.object.ts,variable.other.constant.property.ts,variable.other.property.ts)are mapped to the
variablecustomType — without that, tokens insideuseTranslate(NS.K)arrive withcustomType: undefinedand bypassparsing entirely.
Scope and limitations
import { NS } from './namespaces'is still warned. Following imports is a fair chunk more work (module
resolution, alias paths, ts-morph or equivalent) and explicitly out
of scope for this PR.
NS.KEY). Deeper paths stillwarn.
Verification
npm run test:unit— 597 passed (was 583; +14 new tests).test/unit/extractor/react/useTranslate.test.ts— 116 tests.test/unit/extractor/react/tComponent.test.ts— 42 tests.files were modified).
Namespaces.tsx:3 keys extracted, namespace
namespaced, zero warnings.Test plan
shape.
paths.
description and the new
extractConstants.tsdoc comment).Summary by CodeRabbit
New Features
Bug Fixes
Tests