Fix mixed index signatures lose their broad constraint - #7059
Conversation
fubhy
added
the
audit
🦋 Changeset detectedLatest commit: 99934ba The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
This PR currently adds a failing regression test but does not include the implementation fix. The test correctly demonstrates the bug, but the source change in toJsonSchemaDocument.ts is still needed before this can land.
Reviewed changes
- Added a focused regression test in
packages/effect/test/schema/representation/toJsonSchemaDocument.test.tsthat compiles anObjectsrepresentation with both a broad string index signature and a template-literal index signature, asserting that the resulting JSON Schema has bothpatternPropertiesandadditionalProperties.
⚠️ Implementation fix is missing
The PR description accurately identifies the bug in packages/effect/src/internal/schema/toJsonSchemaDocument.ts at the Objects branch: after assigning out.patternProperties, the code unconditionally deletes out.additionalProperties, which drops the broad index signature. The fix must retain additionalProperties when it was set by a broad index signature, while continuing to omit the default false for object representations that have only pattern signatures.
Be careful not to regress the existing pure-pattern test at packages/effect/test/schema/representation/toJsonSchemaDocument.test.ts:698, which expects { type: "object", patternProperties: { "^a": { type: "string" } } } without additionalProperties.
Technical details
# Mixed index signature fix
## Affected sites
- `packages/effect/src/internal/schema/toJsonSchemaDocument.ts:391-393` — unconditional `delete out.additionalProperties` after emitting `patternProperties`.
## Required outcome
- When an `Objects` representation contains a broad string index signature and one or more pattern index signatures, the emitted JSON Schema must keep the broad type in `additionalProperties` and the pattern types in `patternProperties`.
- When there is no broad index signature, the current behavior of omitting `additionalProperties` (which is the default `false`) must remain unchanged so existing tests keep passing.
## Suggested approach
Track whether a broad index signature contributed to `out.additionalProperties` (e.g. `let hasBroadSignature = false` and set it when `patterns.length === 0`). Then delete `additionalProperties` only when `!hasBroadSignature` after `patternProperties` are emitted.@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Added a changeset describing the fix as an
effectpatch. - Implemented the fix in
packages/effect/src/internal/schema/toJsonSchemaDocument.tsby tracking whether a broad index signature contributed toout.additionalProperties;additionalPropertiesis now only deleted whenpatternPropertiesare emitted if no broad signature exists, preserving the broad constraint for mixed index signatures while keeping pure-pattern behavior unchanged.
The prior review's main concern — the missing implementation fix — is now addressed. Validation passed: the focused toJsonSchemaDocument test file, the related toJsonSchemaMultiDocument test file, pnpm --filter effect check, and pnpm lint-fix all succeed.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
2 participants
Summary
Exporting an object representation with both a broad string index signature and a template-literal index signature drops the broad constraint whenever
patternPropertiesis emitted. Non-pattern keys then become unconstrained, and pattern keys no longer satisfy both source index constraints.Note
This PR includes the focused regression test and the implementation fix.
Mixed index signatures lose their broad constraint
Module:
effect/internal/schema/toJsonSchemaDocumentAudit ID:
effect-bc04d519737af4efSeverity / confidence: medium / high
What happens
Exporting an object representation with both a broad string index signature and a template-literal index signature drops the broad constraint whenever
patternPropertiesis emitted. Non-pattern keys then become unconstrained, and pattern keys no longer satisfy both source index constraints.Why it happens
The compiler stores a patternless string signature in
out.additionalPropertiesand patterned signatures inpatternProperties. After assigningout.patternProperties, it unconditionally deletesout.additionalProperties, erasing the previously compiled broad signature.Expected behavior
SchemaRepresentation.toJsonSchemaDocumentmust encode simultaneous broad and patterned object index signatures conjunctively, retaining the broad schema inadditionalPropertiesalongsidepatternProperties.Relevant implementation
These links and excerpts are pinned to audit base
17f0b91a243ccfe4a38d27debdc983adf434e738.packages/effect/src/internal/schema/toJsonSchemaDocument.ts:342-402View problematic code at
packages/effect/src/internal/schema/toJsonSchemaDocument.ts:342-391View exact lines on GitHub
Excerpt truncated. Open the complete packages/effect/src/internal/schema/toJsonSchemaDocument.ts:342-402 range.
Reproduction
pnpm test --run packages/effect/test/schema/representation/toJsonSchemaDocument.test.tsObserved failure: Focused contract assertion failed against 17f0b91, demonstrating: Mixed index signatures lose their broad constraint.
Validation
The original failing reproduction now passes with the implementation fix:
pnpm test --run packages/effect/test/schema/representation/toJsonSchemaDocument.test.tsAudit provenance
17f0b91a243ccfe4a38d27debdc983adf434e73817f0b91a243ccfe4a38d27debdc983adf434e738effect-bc04d519737af4ef99934ba49Closes EFF-492