fix(mock): handle nullable object schemas and per-import strict kinds#3617
Merged
melloware merged 6 commits intoJun 18, 2026
Merged
Conversation
Strip schema-level null from nullable object mocks when nonNullable is set. Resolve refs and oneOf branches when classifying strict mock schema kinds. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds nullable-at-root detection ( ChangesNullable Object Schema Support
Sequence Diagram(s)sequenceDiagram
participant ClassifySchema as classifyStrictMockSchemaType
participant RefResolver as resolveRef
participant GetDecl as getStrictMockTypeDeclaration
participant MakeMock as getMockObject
Note over ClassifySchema,MakeMock: Strict Mock Type Flow with Context
rect rgba(100, 200, 200, 0.5)
ClassifySchema->>RefResolver: resolve $ref if context provided
RefResolver-->>ClassifySchema: resolved schema
ClassifySchema->>ClassifySchema: recursively classify resolved schema
ClassifySchema-->>GetDecl: schema kind
end
rect rgba(150, 150, 200, 0.5)
GetDecl->>GetDecl: check schemaNullableAtRoot
GetDecl-->>GetDecl: emit type with optional | null union
end
rect rgba(200, 150, 150, 0.5)
MakeMock->>MakeMock: filter null types if nonNullable
MakeMock->>MakeMock: generate properties/additionalProperties
MakeMock->>MakeMock: wrap with faker.helpers.arrayElement if nullable
MakeMock-->>MakeMock: return {value, nullWrapped}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/mock/src/faker/getters/object.ts (2)
174-289:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap only complete object expressions and preserve
nullWrapped.For
allOf, this branch returns a property fragment without braces, sogetNullable(value, ...)can wrapfoo: baras if it were a full expression. Also mark the result asnullWrappedwhen this branch adds the null choice, so parent property assembly does not add another null wrapper.🐛 Proposed fix
+ const returnsObjectExpression = + !combine || combine.separator === 'oneOf' || combine.separator === 'anyOf'; let value = - !combine || combine.separator === 'oneOf' || combine.separator === 'anyOf' - ? '{' - : ''; + returnsObjectExpression ? '{' : ''; @@ value += propertyScalars.join(', '); - value += - !combine || combine.separator === 'oneOf' || combine.separator === 'anyOf' - ? '}' - : ''; + value += returnsObjectExpression ? '}' : ''; + + const nullWrapped = + returnsObjectExpression && + isNullableSchema(schemaItem) && + !mockOptions?.nonNullable; return { - value: getNullable( - value, - isNullableSchema(schemaItem), - mockOptions?.nonNullable, - ), + value: getNullable(value, nullWrapped), imports, name: schemaItem.name, includedProperties, + nullWrapped, };🤖 Prompt for 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. In `@packages/mock/src/faker/getters/object.ts` around lines 174 - 289, The return statement at the end of this object building block incorrectly calls getNullable on value in all cases, but this should only happen when value is a complete object expression with braces. Move the getNullable call inside a conditional block that only executes when !combine or combine.separator is oneOf or anyOf (the cases where braces are added). Additionally, the function needs to return nullWrapped set to true when getNullable is applied to indicate that null wrapping has already been handled at this level, preventing parent property assembly from adding another null wrapper. For the allOf case where combine exists without these specific separators, return the value directly without getNullable and set nullWrapped to false.
296-337:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply root nullability to dictionary and empty-object returns.
nullable: trueobject schemas withoutpropertiesreach theadditionalPropertiesor fallback branches, so they still always emit an object whennonNullableis false. Reuse the same root nullable wrapper for these complete object values.🐛 Proposed fix
if (itemAdditionalProperties) { + const nullWrapped = + isNullableSchema(schemaItem) && !mockOptions?.nonNullable; + if (itemAdditionalProperties === true) { - return { value: `{}`, imports: [], name: schemaItem.name }; + return { + value: getNullable(`{}`, nullWrapped), + imports: [], + name: schemaItem.name, + nullWrapped, + }; } @@ - return { + const value = `{ + [${DEFAULT_OBJECT_KEY_MOCK}]: ${resolvedValue.value} + }`; + + return { ...resolvedValue, - value: `{ - [${DEFAULT_OBJECT_KEY_MOCK}]: ${resolvedValue.value} - }`, + value: getNullable(value, nullWrapped), + nullWrapped, }; } - return { value: '{}', imports: [], name: schemaItem.name }; + const nullWrapped = isNullableSchema(schemaItem) && !mockOptions?.nonNullable; + return { + value: getNullable('{}', nullWrapped), + imports: [], + name: schemaItem.name, + nullWrapped, + }; }🤖 Prompt for 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. In `@packages/mock/src/faker/getters/object.ts` around lines 296 - 337, The complete object values returned in the `itemAdditionalProperties` handling block (both when it is true and when it contains resolved properties) and in the fallback return statement do not apply root nullability wrapping. When a schema is nullable and nonNullable is false, these complete object values should be wrapped with the same root nullable wrapper used elsewhere in the code. Apply the nullable wrapper to the final computed `value` in all three return statements: the early return when itemAdditionalProperties is true, the return after resolving additionalProperties, and the fallback empty object return.
🤖 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 `@packages/mock/src/mock-types.ts`:
- Around line 160-210: The function `resolveStrictMockSchemaForTypeName`
compares the received typeName (which may be an alias) against the resolved
import name, but resolveRef returns imports with only bare names and no alias
field. When the outer import uses an alias, the comparison at lines 187 and 201
fails because branchName and refName only contain the bare import name. Update
both comparison points to check if typeName matches either the resolved import's
bare name OR the resolved import's alias (if available), ensuring the function
correctly matches aliased type names by checking both forms of the reference
name.
- Around line 133-138: In the mappedType constant construction, the issue is
that Required<${typeName}> will drop all object keys when typeName is a union
type like Type | null. Fix this by wrapping typeName with NonNullable before
applying Required, changing the mapped type definition from `{\n [K in keyof
Required<${typeName}>]: NonNullable<Required<${typeName}>[K]>;\n}` to use
NonNullable<${typeName}> inside the Required call, so that the Required utility
extracts keys from the actual object structure rather than from the union that
includes null. Additionally, update the test expectation at mock-types.test.ts
line 166 to reflect the corrected output format.
---
Outside diff comments:
In `@packages/mock/src/faker/getters/object.ts`:
- Around line 174-289: The return statement at the end of this object building
block incorrectly calls getNullable on value in all cases, but this should only
happen when value is a complete object expression with braces. Move the
getNullable call inside a conditional block that only executes when !combine or
combine.separator is oneOf or anyOf (the cases where braces are added).
Additionally, the function needs to return nullWrapped set to true when
getNullable is applied to indicate that null wrapping has already been handled
at this level, preventing parent property assembly from adding another null
wrapper. For the allOf case where combine exists without these specific
separators, return the value directly without getNullable and set nullWrapped to
false.
- Around line 296-337: The complete object values returned in the
`itemAdditionalProperties` handling block (both when it is true and when it
contains resolved properties) and in the fallback return statement do not apply
root nullability wrapping. When a schema is nullable and nonNullable is false,
these complete object values should be wrapped with the same root nullable
wrapper used elsewhere in the code. Apply the nullable wrapper to the final
computed `value` in all three return statements: the early return when
itemAdditionalProperties is true, the return after resolving
additionalProperties, and the fallback empty object return.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: feb7d7ea-58aa-4280-8a5e-4ab4c1190a3b
📒 Files selected for processing (6)
packages/mock/src/faker/getters/object.test.tspackages/mock/src/faker/getters/object.tspackages/mock/src/faker/index.test.tspackages/mock/src/mock-types.test.tspackages/mock/src/mock-types.tspackages/mock/src/msw/index.ts
Skip root-level null randomization when building object fragments for combineSchemasMock so oneOf merges stay valid TypeScript. Update the circular mock snapshot for nullable required child refs. Co-authored-by: Cursor <cursoragent@cursor.com>
Match aliased oneOf imports against resolved bare schema names, use NonNullable inside strict mapped types, and set nullWrapped on root nullable object mocks including additionalProperties fallbacks. Co-authored-by: Cursor <cursoragent@cursor.com>
These snapshots were generated locally with v8.16.0 and have no matching test config; they fail the snapshot version check against master v8.17.0. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mock/src/faker/getters/object.ts (1)
134-186:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the
{}branch for propertyless['object', 'null']schemas.This branch handles OpenAPI 3.1 nullable objects before the fallback wrapper at Lines 389-396. The current test suite still locks the no-property
type: ['object', 'null']case tofaker.helpers.arrayElement([null,]), while the newnullable: trueequivalent returnsfaker.helpers.arrayElement([{}, null]). That leaves the two nullable-object syntaxes generating different top-level mocks and drops the valid non-null{}branch for the OpenAPI 3.1 form.🐛 Possible localized fix for the no-property object/null union
if (Array.isArray(itemType)) { const nonNullTypes = mockOptions?.nonNullable ? itemType.filter((type) => type !== 'null') : itemType; + if ( + !mockOptions?.nonNullable && + itemType.length === 2 && + itemType.includes('object') && + itemType.includes('null') && + !itemProperties && + !itemAdditionalProperties + ) { + const { value: finalValue, nullWrapped } = wrapRootNullableObjectValue( + '{}', + { ...schemaItem, type: 'object', nullable: true } as MockSchemaObject & + Record<string, unknown>, + mockOptions, + combine, + ); + + return { + value: finalValue, + nullWrapped, + imports: [], + name: schemaItem.name, + }; + } + if (nonNullTypes.length === 0) { return { value: 'null', imports: [], name: schemaItem.name }; }🤖 Prompt for 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. In `@packages/mock/src/faker/getters/object.ts` around lines 134 - 186, The combineSchemasMock call for union types like ['object', 'null'] with no properties is not preserving the empty object branch. When nonNullTypes contains 'object' but the schemaItem has no properties or required fields, ensure that the mapped anyOf array includes a valid empty object entry instead of collapsing it. Check the logic where nonNullTypes.map creates the anyOf entries and add special handling to preserve the {} option for propertyless object types, so that ['object', 'null'] generates faker.helpers.arrayElement([{}, null]) instead of just [null].
🤖 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 `@packages/mock/src/faker/getters/object.ts`:
- Around line 382-386: The nullWrapped property in the returned object is
incorrectly propagating the nested property's nullWrapped state by using
`nullWrapped || resolvedValue.nullWrapped`. Since resolvedValue.nullWrapped
describes the additional-property value and not the dictionary object being
returned, remove the `|| resolvedValue.nullWrapped` portion from the nullWrapped
assignment in the return statement so that only the nullWrapped parameter is
used for the containing object's nullWrapped property.
---
Outside diff comments:
In `@packages/mock/src/faker/getters/object.ts`:
- Around line 134-186: The combineSchemasMock call for union types like
['object', 'null'] with no properties is not preserving the empty object branch.
When nonNullTypes contains 'object' but the schemaItem has no properties or
required fields, ensure that the mapped anyOf array includes a valid empty
object entry instead of collapsing it. Check the logic where nonNullTypes.map
creates the anyOf entries and add special handling to preserve the {} option for
propertyless object types, so that ['object', 'null'] generates
faker.helpers.arrayElement([{}, null]) instead of just [null].
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d5d297ba-cca3-4b31-920c-c62e90bbac20
⛔ Files ignored due to path filters (34)
tests/__snapshots__/mock/issue-3525-multi/endpoints.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3525-multi/model/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3525-oas31/endpoints.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3525-oas31/model/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3525-widget-mock-strict/endpoints.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3525/endpoints.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3525/model/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/index.msw.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/pets/pets.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/pets/pets.msw.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/pets/pets.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/schemas/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/schemas/index.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/schemas/pet.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/index.msw.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/pets/pets.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/pets/pets.msw.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/pets/pets.service.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/schemas/index.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/schemas/pet.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-fetch/pets/pets/pets.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-fetch/pets/pets/pets.msw.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-fetch/schemas/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-multi-fetch/schemas/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-multi-fetch/store/store/store.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-multi-fetch/store/store/store.msw.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3574-strict-mock-tags-split/pets/pets/pets.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3574-strict-mock-tags-split/pets/pets/pets.msw.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/pets/pets/pets.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/schemas/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/index.faker.tsis excluded by!**/__snapshots__/**tests/__snapshots__/mock/issue-3590/model/index.faker.tsis excluded by!**/__snapshots__/**
📒 Files selected for processing (4)
packages/mock/src/faker/getters/object.test.tspackages/mock/src/faker/getters/object.tspackages/mock/src/mock-types.test.tspackages/mock/src/mock-types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/mock/src/mock-types.test.ts
- packages/mock/src/mock-types.ts
Stop propagating nested nullWrapped from additionalProperties values and
emit {} for propertyless type: ['object', 'null'] unions.
Co-authored-by: Cursor <cursoragent@cursor.com>
…l union
NullableAnyObject now emits faker.helpers.arrayElement([{}, null])
instead of [null] only.
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
melloware
approved these changes
Jun 18, 2026
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
2 participants
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.
Summary
type: object+nullable: true, ortype: ['object', 'null']): whennonNullableis enabled, factories no longer emit top-levelnullbranches that conflict with{Schema}Mockreturn types.$reftargets and matchingoneOf/anyOfbranches, instead of applying the response wrapper schema to every imported type.$reftargets inclassifyStrictMockSchemaTypeso enum refs emit alias mocks instead of object mapped types.Test plan
packages/mock/src/mock-types.test.tspackages/mock/src/faker/getters/object.test.tspackages/mock/src/faker/index.test.tsbun run lintEOF
Summary by CodeRabbit
New Features
Bug Fixes
nonNullableoverrides are respected, andnullWrappedbehavior is propagated correctly.Tests
$refresolution, andoneOfresponse classification, plus strict-mock expectations fortype: ['object', 'null'].