fix(transformers): use symbols for additionalProperties key variables#3841
Conversation
Replace literal string 'key' with plugin.symbol('key') in the
additionalProperties for-loop generation. This ensures the planner
assigns unique names (key, key2, key3, ...) when nested or sibling
additionalProperties produce multiple for-loops, preventing the
TypeScript error caused by duplicate variable names in nested scopes.
Closes #3838
|
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: d880caa The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3841 +/- ##
==========================================
- Coverage 39.58% 39.58% -0.01%
==========================================
Files 532 532
Lines 19580 19581 +1
Branches 5829 5835 +6
==========================================
Hits 7751 7751
- Misses 9581 9582 +1
Partials 2248 2248
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@hey-api/codegen-core
@hey-api/json-schema-ref-parser
@hey-api/nuxt
@hey-api/openapi-ts
@hey-api/shared
@hey-api/spec-types
@hey-api/types
@hey-api/vite-plugin
commit: |
|
@pullfrog actually, the snapshot that changed shouldn't have changed. This is because both keys are within sibling blocks and therefore there's no conflict. This will be either an issue with the DSL incorrectly declaring scopes, or the core package incorrectly grouping these scopes so they conflict. Can you figure out what's the root cause first? After that, I'll give you the instructions how to resolve this issue. |
|
Identified root cause: Task list (3/3 completed)
|
|
Root cause identified — it's the DSL incorrectly declaring scopes. The issue is in override analyze(ctx: AnalysisContext): void {
ctx.analyze(this._condition);
ctx.analyze(this._iterableOrUpdate);
ctx.analyze(this._variableOrInit); // ← registers 'key' in the PARENT scope
ctx.pushScope(); // ← for-loop scope starts here
try {
super.analyze(ctx); // DoMixin pushes body scope
} finally {
ctx.popScope();
}
}The iteration variable ( The consequence: when the planner calls The fix should be moving override analyze(ctx: AnalysisContext): void {
ctx.analyze(this._condition);
ctx.analyze(this._iterableOrUpdate);
ctx.pushScope();
ctx.analyze(this._variableOrInit); // now scoped to the for-loop block
try {
super.analyze(ctx);
} finally {
ctx.popScope();
}
}Ready for your instructions on how to proceed. |
|
@pullfrog that's the correct fix, apply it and then run tests to confirm everything works |
|
Applied the root cause fix: moved Task list (5/5 completed)
|
Move ctx.analyze(this._variableOrInit) after ctx.pushScope() so the
iteration variable is registered in the for-loop's own scope rather
than the parent scope. This matches JS semantics where `for (const x of ...)`
scopes `x` to the loop block, preventing false conflicts between sibling loops.
Reverts the plugin.symbol('key') workaround since the correct scoping
makes it unnecessary — sibling for-loops no longer conflict.
|
@pullfrog this commit is correct but we also want to keep that symbol change in the transformers plugin. Add that back |
|
Re-added the Task list (3/3 completed)
|
Fixes #3838
Problem
The
@hey-api/transformersplugin generates duplicatekeyvariable names when nestedadditionalPropertiesproduce multiple for-loops. This causes TypeScript compilation errors:Fix
Replace the literal string
'key'withplugin.symbol('key')so the planner assigns unique names (key,key2,key3, ...) when there are conflicts. Each invocation of theadditionalPropertieshandling block creates its own symbol, and the planner's name-conflict resolution ensures distinctness across nested and sibling scopes.