Fix 5331: resolve a null-first type list to its first non-null type - #5428
heath-freenome wants to merge 2 commits into
Conversation
Size limit reportSizes are minified + brotli, measured by size-limit at A package listed twice is measured both as installed and, on the second row, as its own code with its dependencies excluded. Peer dependencies are always excluded.
|
f2a662d to
10d01d6
Compare
| // oxlint-disable-next-line prefer-destructuring | ||
| type = type[0]; | ||
| } | ||
| type = type.find((t) => t !== 'null') ?? type[0]; |
There was a problem hiding this comment.
Spurious ui:required errors on a non-object value of a null-first list. getUiRequiredErrorSchema() (packages/utils/src/schema/getUiRequiredErrorSchema.ts:186) walks into properties whenever getSchemaType(retrieved) === 'object'. A ['null', 'object', 'string'] used to resolve to null here, so the walk stopped. It now resolves to object, so the walk goes down into the children even when the value is a string or null.
Repro: schema { type: 'object', properties: { v: { type: ['null', 'object', 'string'], properties: { a: { type: 'string' } } } } }, uiSchema { v: { a: { 'ui:required': true } } }, formData { v: 'hello' }. childParentPresent is true and data.a is undefined, so you get must have required property 'a' at v.a and submit is blocked, even though 'hello' is valid.
omitExtraData() got getPruningType() for this exact value-vs-resolved-type mismatch, but this sibling consumer didn't. It should resolve the type from the value it walks too.
There was a problem hiding this comment.
Fixed in 922b555. getUiRequiredErrorSchema() now gets the type from the value through the new getSchemaTypeForValue(), so a string under ['null', 'object', 'string'] isn't walked into. Your repro is a test in getUiRequiredErrorSchema.test.ts. A null is still read by the resolved type, because ObjectField renders an object field's properties even when it holds null.
| - `logOnce()` compares a plain-object or array `error` by its JSON, so two different plain-object payloads logged with the same message are no longer taken for one, and the `logUnsupportedDefaultForEnum()` error spells an object default out as JSON rather than `[object Object]`. `enumOptionValueLabel()` spells a value the same way, so a value reads the same in an option label as in a warning: a `Date`, a `Map` or a nested `BigInt` is spelled as `String()` spells it, and a circular plain object or array no longer throws. ([#5382](https://github.com/rjsf-team/react-jsonschema-form/pull/5382)) | ||
| - **BREAKING CHANGE:** `UiSchemaDefinitions<S, F>` drops its `T` parameter and types every entry as `UiSchema<unknown, S, F>`, the type `UiSchema`'s `ui:definitions` key now uses too. A definition applies to whichever field references it, so typing it by the root form's data rejected the nested keys of the field it describes ([#5372](https://github.com/rjsf-team/react-jsonschema-form/pull/5372)) | ||
| - Fixed `getDateTimeLocalValue()` returning `undefined` for an epoch number or a `Date`: a finite number or a valid `Date` is now converted to its UTC ISO string for `date-time`/`datetime`, to the day it names for a `date` that is a UTC midnight, and to local wall-clock time for any other format. Its result also gains `requiresOffset`, true for `date-time`/`datetime` ([#5393](https://github.com/rjsf-team/react-jsonschema-form/issues/5393)) | ||
| - Fixed `getSchemaType()` returning `null` for a `type` list of three or more entries that starts with `null`: it returns the first type in the list other than `null`, as it already did for a two-entry list, and `null` only for a list naming nothing else ([#5331](https://github.com/rjsf-team/react-jsonschema-form/issues/5331)) |
There was a problem hiding this comment.
This also changes getDefaultFormState() for any null-first list, and neither changelog entry says so. getDefaultBasedOnSchemaType() switches on getSchemaType() (getDefaultFormState.ts:892). So for a required ['null', 'boolean', 'string'] with no default, the form now starts at false (the case 'boolean' branch), where before it was left unset. A null-first list naming object or array now gets object/array defaults computed for it.
The upgrade guide mentions this, but these CHANGELOG-v7.md bullets and the PR body only cover rendering and object/array defaults. A form that relied on the field starting empty now gets a value it never set.
There was a problem hiding this comment.
Added in 922b555. The getSchemaType() bullet now says getDefaultFormState() follows it, so a null-first list gets that type's defaults. The PR description gives both examples: false for a required ['null', 'boolean', 'string'] and object defaults for ['null', 'object', 'string'].
| // oxlint-disable-next-line prefer-destructuring | ||
| type = type[0]; | ||
| } | ||
| type = type.find((t) => t !== 'null') ?? type[0]; |
There was a problem hiding this comment.
Optional Data Controls now turn on for a union field. getOptionalDataControlsType() (shouldRenderOptionalField.ts:46) returns getSchemaType(schema). A ['null', 'object', 'string'] (or ['null', 'array', ...]) now answers object/array instead of null. So enableOptionalDataFieldForType: ['object'] now enables the optional-data controls for a field that allows several types, which contradicts the isOptionalDataControlsType() docstring ("A field whose type is a list of several types is never one of them"). getUiRequiredErrorSchema() uses the same predicate to gate its walk.
This was already true for lists that don't start with null (['object', 'string']), but this change brings null-first lists into it. Either check getUnionTypes() there, or update the docstring.
There was a problem hiding this comment.
Fixed in 922b555 with getUnionTypes(). getOptionalDataControlsType() now returns getUnionTypes(schema) ?? getSchemaType(schema), so a list of several non-null types comes back whole and isOptionalDataControlsType() rejects it, as its docstring says. That covers ['object', 'string'] too. getSchemaTypesForXxxOf() does the same for each option.
| // oxlint-disable-next-line prefer-destructuring | ||
| type = type[0]; | ||
| } | ||
| type = type.find((t) => t !== 'null') ?? type[0]; |
There was a problem hiding this comment.
Reuse: this expression is exactly getDefaultType() in packages/core/src/components/fields/FallbackField.tsx:53 (types.find((aType) => aType !== 'null') ?? types[0]). The PR body even calls it "the rule FallbackField already uses". Pulling it into one small util that both call keeps the "first non-null type" rule from drifting between the two.
There was a problem hiding this comment.
Done in 922b555. FallbackField's getDefaultType() is now getSchemaType({ type: types }). The rule also gained the known-type filter from your later comment, so the two can't drift.
| // oxlint-disable-next-line prefer-destructuring | ||
| type = type[0]; | ||
| } | ||
| type = type.find((t) => t !== 'null') ?? type[0]; |
There was a problem hiding this comment.
Simplification: after this line, getSchemaType() can't return an array (that was also true before, since the old branches only returned type[0] or find). Its declared return type is still string | string[] | undefined, though, which keeps dead code alive in callers: Array.isArray(schemaType) ? schemaType[0] : ... in SchemaField.getFieldComponent() (SchemaField.tsx:222, a function this PR touches) and the .flat() in getSchemaTypesForXxxOf(). Narrowing the return type to string | undefined lets both go.
There was a problem hiding this comment.
Done in 922b555. The return type is string | undefined. The Array.isArray(schemaType) branch in SchemaField and the .flat() in getSchemaTypesForXxxOf() are gone.
| * @param value - The form data being pruned | ||
| * @returns - The type to prune `value` as | ||
| */ | ||
| function getPruningType<S extends StrictRJSFSchema>(schema: S, value: unknown) { |
There was a problem hiding this comment.
Altitude: value-aware type resolution lives only in omitExtraData. Now that more lists resolve to a container type, other consumers branch on getSchemaType() while handed a value of another listed type. getUiRequiredErrorSchema() (walks object children of a string value) and getDefaultBasedOnSchemaType() (computes object defaults over a string rawFormData) both do this. Consider moving getPruningType() into @rjsf/utils as a shared helper, e.g. getSchemaTypeForValue(schema, value), and using it at those call sites. Otherwise each consumer has to rediscover this mismatch on its own.
There was a problem hiding this comment.
Done in 922b555. getPruningType() became getSchemaTypeForValue(schema, value) in @rjsf/utils. It's used by omitExtraData(), getUiRequiredErrorSchema() and FallbackField's initial type. I didn't change getDefaultBasedOnSchemaType(): computeDefaults() already checks isObject(rawFormData), so { v: 'hello' } under ['null', 'object', 'string'] with a property default stays { v: 'hello' }. I confirmed that at the new head.
| */ | ||
| function getPruningType<S extends StrictRJSFSchema>(schema: S, value: unknown) { | ||
| const { type } = schema; | ||
| if (Array.isArray(type) && value !== undefined) { |
There was a problem hiding this comment.
Dead guard: value !== undefined can never be false here. The only caller is omit(), which returns at the top when source === undefined, before it reaches getPruningType(localSchema, source). Drop the check, or say in the docblock why it's needed if it's meant to protect future callers. As written, it suggests guessType(undefined) === 'null' is a live case.
There was a problem hiding this comment.
getPruningType() is gone. Its replacement, getSchemaTypeForValue(), is a public util with other callers, so the undefined guard stays there. The docblock now says why: guessType(undefined) reports null, and an undefined value means there's no value, not a null one. A test covers it.
| // oxlint-disable-next-line prefer-destructuring | ||
| type = type[0]; | ||
| } | ||
| type = type.find((t) => t !== 'null') ?? type[0]; |
There was a problem hiding this comment.
A null-first list with a string-only ui:widget now crashes the whole form.
{ type: ['null', 'number', 'string'] } with uiSchema: { 'ui:widget': 'textarea' } used to resolve to null and render NullField. It now resolves to number, so NumberField → StringField → getWidget() looks textarea up in the number widget map and throws No widget 'textarea' for type 'number' in schema: {"type":["null","number","string"]}. I reproduced this with createFormComponent at 3a8a5f9. The render throws, so the whole Form goes down, not just that field. Any string-only alias (textarea, email, password, ...) on a ['null','number'|'integer'|'boolean', 'string', ...] list does the same, as does a number-only alias (updown, range) on ['null','string','number'].
['number', 'string'] already behaved this way, but this PR exposes null-first lists that rendered before. At minimum it needs a changelog/migration note. Better: when the named alias isn't in the resolved type's map, getWidget() could fall back to another type in the list that does have it.
There was a problem hiding this comment.
Fixed in 922b555, in two places:
getWidget()accepts a widget of any non-arraytype the list names. An array widget still throws unless the list resolves toarray, because onlyArrayFieldhands it a list of items.SchemaFieldrenders the field of the listed type that has the widget when the resolved type has none by that name. Without this,NumberFieldwould render thetextareaand save'007'as7.textarea/emailon['null', 'number', 'string']now renderStringField,updownon['null', 'string', 'number']rendersNumberField, andcheckboxon['number', 'boolean']rendersBooleanField. Each has a test.
A widget given by its registered name ('TextareaWidget') or as a component doesn't trigger the switch yet. That's listed as a known gap in the description.
|
|
||
| // A select holds one of its constants as a whole, so an `object` or `array` one has no contents to prune | ||
| const type = isWholeValueSelect<S>(localSchema) ? undefined : getSchemaType<S>(localSchema); | ||
| const type = isWholeValueSelect<S>(localSchema) ? undefined : getPruningType<S>(localSchema, source); |
There was a problem hiding this comment.
omitExtraData() now drops the whole object value of any type list without properties, including lists that have no null at all.
getPruningType() prunes an object value as object whenever the list includes object, and handleObject() treats a missing additionalProperties as false. So a properties-less object comes out {}, and setProperty() then drops the key. I checked base and PR head with the same input:
schema of v |
formData | base (9af3757) | this PR |
|---|---|---|---|
{ type: ['string', 'object'] } |
{ v: { k: 1 } } |
{ v: { k: 1 } } |
{} |
{ type: ['null', 'object', 'string'] } |
{ v: { k: 1 } } |
{ v: { k: 1 } } |
{} |
With omitExtraData/liveOmit on, an object that came in through formData is now silently removed on submit. The first row isn't null-first at all, so the change reaches well past #5331. The changelog bullet only describes what is now kept. Either limit the value-type override to keeping non-container values (and let objects/arrays prune as before), or call out the new pruning of ['…', 'object'] values in the changelog.
There was a problem hiding this comment.
I kept this behavior on purpose. An object value of a list naming object is now pruned exactly like one under a plain { type: 'object' }. I checked at the new head: { v: { k: 1 } } gives {} for { type: 'object' }, ['string', 'object'] and ['null', 'object', 'string'] alike. A properties-less object schema means no properties are allowed, and omitExtraData already takes it that way for a single type. The old result for the union came from resolving to string and skipping object pruning altogether. I've called it out under side effects in the PR description, ['string', 'object'] included. I can also add it to the changelog if you'd like it there.
| // oxlint-disable-next-line prefer-destructuring | ||
| type = type[0]; | ||
| } | ||
| type = type.find((t) => t !== 'null') ?? type[0]; |
There was a problem hiding this comment.
"First non-null" picks an unrecognized type name over a renderable one.
For ['null', 'foo', 'string'], find((t) => t !== 'null') returns foo. COMPONENT_TYPES has no foo, so SchemaField falls to FallbackField, and with the fallback UI off that renders the unsupported-field box. string is right there in the list, and getKnownTypes()/getUnionTypes() already filter to JSON_SCHEMA_TYPES. Filtering to known, non-null types first (type.find((t) => t !== 'null' && JSON_SCHEMA_TYPES.includes(t)) ?? type[0]) would make the resolution agree with getKnownTypes().
There was a problem hiding this comment.
Done in 922b555. getSchemaType() now picks the first non-null type in JSON_SCHEMA_TYPES, then the first non-null type, then type[0]. So ['null', 'foo', 'string'] resolves to string and ['null', 'foo'] still resolves to foo. Both are test cases.
| - `logOnce()` compares a plain-object or array `error` by its JSON, so two different plain-object payloads logged with the same message are no longer taken for one, and the `logUnsupportedDefaultForEnum()` error spells an object default out as JSON rather than `[object Object]`. `enumOptionValueLabel()` spells a value the same way, so a value reads the same in an option label as in a warning: a `Date`, a `Map` or a nested `BigInt` is spelled as `String()` spells it, and a circular plain object or array no longer throws. ([#5382](https://github.com/rjsf-team/react-jsonschema-form/pull/5382)) | ||
| - **BREAKING CHANGE:** `UiSchemaDefinitions<S, F>` drops its `T` parameter and types every entry as `UiSchema<unknown, S, F>`, the type `UiSchema`'s `ui:definitions` key now uses too. A definition applies to whichever field references it, so typing it by the root form's data rejected the nested keys of the field it describes ([#5372](https://github.com/rjsf-team/react-jsonschema-form/pull/5372)) | ||
| - Fixed `getDateTimeLocalValue()` returning `undefined` for an epoch number or a `Date`: a finite number or a valid `Date` is now converted to its UTC ISO string for `date-time`/`datetime`, to the day it names for a `date` that is a UTC midnight, and to local wall-clock time for any other format. Its result also gains `requiresOffset`, true for `date-time`/`datetime` ([#5393](https://github.com/rjsf-team/react-jsonschema-form/issues/5393)) | ||
| - Fixed `getSchemaType()` returning `null` for a `type` list of three or more entries that starts with `null`: it returns the first type in the list other than `null`, as it already did for a two-entry list, and `null` only for a list naming nothing else ([#5331](https://github.com/rjsf-team/react-jsonschema-form/issues/5331)) |
There was a problem hiding this comment.
The new default-filling for null-first lists isn't in the changelog.
Because these lists now resolve to object/array, getDefaultBasedOnSchemaType() fills them in on load. An optional v: { type: ['null', 'object', 'string'], properties: { a: { type: 'string', default: 'x' } } } with formData {} gave {} on base and gives { v: { a: 'x' } } here (checked at both commits). That value is then submitted even if the user never touched the field. The PR description and the v7 upgrade guide mention it, but this getSchemaType() bullet and the @rjsf/core bullet don't. A user scanning CHANGELOG-v7.md would miss a change to initial form data.
There was a problem hiding this comment.
Added in 922b555. The getSchemaType() bullet now says getDefaultFormState() follows it, and the PR description has your { v: { a: 'x' } } example.
| * @param value - The form data being pruned | ||
| * @returns - The type to prune `value` as | ||
| */ | ||
| function getPruningType<S extends StrictRJSFSchema>(schema: S, value: unknown) { |
There was a problem hiding this comment.
Reuse: this re-implements getInitialType() from FallbackField.
packages/core/src/components/fields/FallbackField.tsx getInitialType() already does guessType(formData) → types.includes(dataType) → dataType === 'number' && types.includes('integer'). That's the same matching, so the "which listed type does this value have" rule now lives in two packages that can drift apart. You could move one helper into @rjsf/utils (e.g. getListedTypeOfValue(types, value)), and have FallbackField and omitExtraData() both call it.
There was a problem hiding this comment.
Done in 922b555 as getSchemaTypeForValue(schema, value), which takes the schema instead of a bare list so it can fall back to getSchemaType(). FallbackField's getInitialType() and omitExtraData() both call it now.
| // oxlint-disable-next-line prefer-destructuring | ||
| type = type[0]; | ||
| } | ||
| type = type.find((t) => t !== 'null') ?? type[0]; |
There was a problem hiding this comment.
Reuse: this expression is now FallbackField's getDefaultType() word for word.
packages/core/src/components/fields/FallbackField.tsx:52-54 is types.find((aType) => aType !== 'null') ?? types[0]. Now that getSchemaType() encodes the same rule, getDefaultType() could go and its call sites could use getSchemaType(). Otherwise the first-non-null rule is stated twice and can drift.
There was a problem hiding this comment.
Done in 922b555: getDefaultType() now just calls getSchemaType().
| */ | ||
| function getPruningType<S extends StrictRJSFSchema>(schema: S, value: unknown) { | ||
| const { type } = schema; | ||
| if (Array.isArray(type) && value !== undefined) { |
There was a problem hiding this comment.
Dead guard: value !== undefined can never be false here. The only caller is omit(), which returns early on source === undefined (the first line of omit()) before it gets here. if (Array.isArray(type)) is enough.
There was a problem hiding this comment.
See the reply above: the guard moved into the public getSchemaTypeForValue(), where an undefined value can reach it, and its docblock says why.
| // type resolves to its first entry, which says nothing about what the others accept, so it keeps the plain text | ||
| // input it had before | ||
| // type resolves to its first non-null entry, which says nothing about what the others accept, so it keeps the plain | ||
| // text input it had before |
There was a problem hiding this comment.
CLAUDE.md: "Don't leave 'before/after' or diff-style commentary in code comments … That belongs in commit messages and PR descriptions, not in code that has to keep reading true after the change ships." (CLAUDE.md → Code comments)
This rewritten comment keeps "so it keeps the plain text input it had before". Suggest saying what the code does now, e.g. "…says nothing about what the others accept, so such a union gets a plain text input".
There was a problem hiding this comment.
Reworded in 922b555: "…resolves to just one of them, which says nothing about what the others accept, so such a union gets a plain text input".
`getSchemaType()` collapsed a two-entry nullable list to its other type but took `type[0]` for anything longer, so `['null', 'string', 'number']` resolved to `null` and `SchemaField` rendered `NullField`: no control to enter a value in, and beside an `anyOf`/`oneOf` an option selector whose every option rendered nothing, since `MultiSchemaField` hands the parent's type list to each option. - `getSchemaType()` now returns the first type other than `null` for any list, and `null` only for a list naming nothing else, matching what `FallbackField` already picks as its initial type - `omitExtraData()` prunes a value of any type a `type` list allows as that type rather than by the one type `getSchemaType()` resolves the list to, so a `['null', 'object', 'string']`, which now resolves to `object`, no longer drops a valid `null` or string, and a `['null', 'object']` no longer drops a `null` - Add `getSchemaType()` cases and `SchemaField` regression tests for the plain and `oneOf` shapes from the issue - Point the `getWidget()` null-type test at a plain `null` select, since a null-first list no longer reaches the null widget map - Update the utility-functions docs, the v7 upgrade guide's `getSchemaType()` and `getWidget()` bullets, and the `SchemaField` and `getInputProps()` comments that described the old resolution Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> # Conflicts: # CHANGELOG-v7.md # Conflicts: # CHANGELOG-v7.md
3a8a5f9 to
e2a58fa
Compare
- Add getSchemaTypeForValue() and use it in omitExtraData(), getUiRequiredErrorSchema() and FallbackField, so a value of another listed type is read as that type - getSchemaType() prefers a type JSON Schema defines and returns string | undefined - getWidget() accepts a widget of any non-array type a type list names; SchemaField renders the field of the listed type that has the widget, so its value isn't cast to the resolved type - getOptionalDataControlsType() returns a list of several types whole - FallbackField reuses the utils rules instead of restating them - Update docs and condense the changelog entries Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
2 participants
Reasons for making this change
Fixes #5331
getSchemaType()collapsed a two-entry nullable list to its other type but tooktype[0]for anything longer, so['null', 'string', 'number']resolved tonulland, withuseFallbackUiForUnsupportedTypeoff,SchemaFieldrenderedNullField: no control to enter a value in. Beside ananyOf/oneOfit rendered an option selector whose every option was blank, sinceMultiSchemaFieldhands the parent's type list to an option that names no type of its own.What changed
@rjsf/utilsgetSchemaType()resolves a list to its first non-null type that JSON Schema defines, then its first non-null type, thennull.['null', 'foo', 'string']resolves tostring, matchinggetKnownTypes(). Its return type narrows tostring | undefined, since it never returned an array.getSchemaTypeForValue(schema, value)(new, exported): the listed type a value has (a number counts as anintegerthe list names), otherwisegetSchemaType(). It's used by:omitExtraData(), so a string ornullheld by a['null', 'object', 'string']is kept rather than droppedgetUiRequiredErrorSchema(), so a string held by that list isn't checked forui:requiredpropertiesFallbackField's initial typegetWidget()accepts a widget of any non-arraytype the list names, so atextareaon a['null', 'number', 'string']no longer throws. Anarraywidget still needs the list to resolve toarray, since onlyArrayFieldhands it a list of items.getOptionalDataControlsType()returns a list of several types whole (getUnionTypes() ?? getSchemaType()), soenableOptionalDataFieldForType: ['object']doesn't turn the controls on for['null', 'object', 'string']. This matches theisOptionalDataControlsType()docstring.@rjsf/coreSchemaFieldrenders the field of the listed type that has the field's widget when the resolved type has none by that name.textarea/emailon['null', 'number', 'string']renderStringFieldand keep'007'as a string.updownon['string', 'number']rendersNumberField, andcheckboxon['number', 'boolean']rendersBooleanField. Without this the resolved type's field would cast the widget's value to its own type.FallbackFieldcallsgetSchemaType()andgetSchemaTypeForValue()instead of restating their rules.Visible side effects
For a null-first list of three or more types:
FieldTemplategetsrjsf-field-<type>instead ofrjsf-field-null.getDefaultFormState()fills in that type's defaults. A required['null', 'boolean', 'string']starts atfalse, and a['null', 'object', 'string']gets its properties' defaults. A value of another listed type already in the form data, such as a string, is left alone.omitExtraData()prunes an object value of any list namingobjectthe way it prunes one under a plain{ type: 'object' }. This includes lists that don't start withnull, such as['string', 'object']. So a list withoutpropertiesoradditionalPropertiesdrops its object value, as{ type: 'object' }already does.Known gaps, left for follow-ups
nullis still checked forui:requiredby the resolved type. A field the user hasn't chosen can raise an error they can't see.'ui:widget': 'TextareaWidget') or as a component doesn't switchSchemaFieldto another listed type's field.getFieldClassNames()/getDisplayLabel()use the resolved type, not the type of the fieldSchemaFieldswitched to.Changelog
CHANGELOG-v7.md:@rjsf/core: one bullet@rjsf/utils: three bullets, one each forgetSchemaType(),getSchemaTypeForValue(), andgetWidget()/getOptionalDataControlsType()The v7 upgrade guide and
utility-functions.mdare updated to match.Tests
getSchemaType(): null-first lists,nullin the middle, unknown type names,['null'],['null', 'null'],[]getSchemaTypeForValue(): listed and unlisted value types,integerfor a number, anundefinedvaluegetWidget(): a widget of another listed type resolves, array widgets throw unless the list resolves toarray, and a widget no listed type has still throwsgetUiRequiredErrorSchema(),shouldRenderOptionalField()andomitExtraData()cases for the union shapes above. TheomitExtraData()cases include anullunder['null', 'object'], which is now kept.SchemaField: both shapes from the issue, plustextarea,email, auuidformat,updownandcheckboxon type lists, each asserting the saved value's typepnpm testpasses in all packages (no snapshot changes);lintandknipare clean.🤖 Generated with Claude Code