Fix declaration emit dropping readonly from as const object literal properties#3933
Conversation
Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/a6a59cb3-682f-426b-8fed-1bddcd23ab2c Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
…properties Fix pseudotypenodebuilder to use the ObjectLiteralExpression node instead of the property name when checking isConstContext. The property name's parent could be a MethodDeclaration which isConstContext doesn't traverse through, causing readonly to be dropped from method shorthands and outer properties in as const objects. Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/a6a59cb3-682f-426b-8fed-1bddcd23ab2c Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
as const object literal properties
There was a problem hiding this comment.
Pull request overview
Fixes a declaration emit regression in the Go port where as const object literals could lose readonly on all properties when the first property was a method shorthand, by ensuring const-context detection starts from the object literal node rather than the first property’s name.
Changes:
- Update pseudo type node building to compute
isConstfrom the containingObjectLiteralExpression(so method shorthands don’t break const-context propagation). - Add a new compiler test + reference baselines covering
as constwith a method shorthand and nested object property. - Update an existing reference baseline (
declarationEmitConstObjectLiteralGenericMethod1) to reflect correct readonly + function-type property emission for const object literal methods.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/checker/pseudotypenodebuilder.go | Fixes const-context detection for pseudo object literal emission by anchoring the check at the object literal node. |
| testdata/tests/cases/compiler/declarationEmitReadonlyAsConst.ts | Adds regression test reproducing the missing-readonly behavior with a leading method shorthand. |
| testdata/baselines/reference/compiler/declarationEmitReadonlyAsConst.types | Reference type baseline for the new test. |
| testdata/baselines/reference/compiler/declarationEmitReadonlyAsConst.symbols | Reference symbol baseline for the new test. |
| testdata/baselines/reference/compiler/declarationEmitReadonlyAsConst.js | Reference JS + .d.ts baseline verifying readonly properties and function-type method emission. |
| testdata/baselines/reference/compiler/declarationEmitConstObjectLiteralGenericMethod1.js | Updates .d.ts reference output to match corrected readonly/function-type emission for const object literal generic methods. |
weswigham
left a comment
There was a problem hiding this comment.
Yep, this looks right - there was just a logic error mismatch between what isConstContext checks and what it was expected to check. It doesn't flow const-ness down into names of declarations, rather it only checks the expression itself, so we need to walk up from name -> member -> expression.
|
@copilot merge main and resolve conflicts and update baselines |
…aration-emit # Conflicts: # testdata/baselines/reference/compiler/declarationEmitConstObjectLiteralGenericMethod1.js Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/ed38f5e8-6c91-49de-be00-980a2f99f522 Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
weswigham
left a comment
There was a problem hiding this comment.
Aside: methods and function-typed members have different behavior - adding readonly swapping which one we printback is... probably a very subtle error (say it with me: "function argument bivariance"). We probably need to support readonly on methods properly to fix that.
origin/main