Skip to content

Commit 6c7c1d7

Browse files
committed
refactor ''/null edge-case to use .transform()
1 parent 3913ab8 commit 6c7c1d7

File tree

1 file changed

+38
-46
lines changed

1 file changed

+38
-46
lines changed

src/yupSchema.js

Lines changed: 38 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,39 @@ const yupSchemas = {
3737
radioOrSelect: (options) =>
3838
string()
3939
.nullable()
40+
.transform((value) => {
41+
if (value === '') {
42+
return undefined; // [1]
43+
}
44+
if (options?.includes(null)) {
45+
return value;
46+
}
47+
return value === null
48+
? undefined // [3]
49+
: value;
50+
51+
/*
52+
[1] @BUG RMT-518 - explanation from PR#18
53+
The "" (empty string) is to keep retrocompatibily with previous version.
54+
The "" does NOT match the JSON Schema specs. In the specs the `oneOf` keyword does not allow "" value by default.
55+
56+
Disallowing "" would be a major BREAKING CHANGE
57+
because previously any string was allowed but now only the options[].value are,
58+
which means we'd need to also exclude "" from being accepted.
59+
This would be a dangerous change as it can disrupt existing UI Form integrations
60+
that might handle empty fields differently ("" vs null vs undefined).
61+
62+
[2] The null also needs to be always allowed for the same reason.
63+
Some consumers (eg Remote) expect empty optional fields to be sent as "null"
64+
even when their JSON Schemas are not created correctly (missing an option with const: null).
65+
This will allow the JSF to still mark the field as valid (false positive)
66+
and let the JSON Schema validator fail.
67+
68+
We'd need to implement a feature_flag/transition deprecation warning
69+
to give devs the time to adapt their integrations before we fix this behavior.
70+
Check the PR#18 and tests for more details.
71+
*/
72+
})
4073
.oneOf(options, ({ value }) => {
4174
return `The option ${JSON.stringify(value)} is not valid.`;
4275
}),
@@ -81,66 +114,25 @@ const getJsonTypeInArray = (jsonType) =>
81114
? jsonType.find((val) => val !== 'null') // eg ["string", "null"] // optional fields - get the head type.
82115
: jsonType; // eg "string"
83116

84-
const getOptionsAllowed = (field) => {
85-
const onlyValues = field.options?.map((option) => option.value);
86-
const optionsBroken = [
87-
...onlyValues,
88-
'', // [1]
89-
];
90-
91-
if (field.required) {
92-
return [
93-
...optionsBroken,
94-
null, // [2]
95-
];
96-
}
117+
const getOptions = (field) => {
118+
const allValues = field.options?.map((option) => option.value);
97119

98120
const isOptionalWithNull =
99121
Array.isArray(field.jsonType) &&
100122
// @TODO should also check the "oneOf" directly looking for "null"
101123
// option but we don't have direct access at this point.
102124
// Otherwise the JSON Schema validator will fail as explained in PR#18
103125
field.jsonType.includes('null');
104-
if (isOptionalWithNull) {
105-
return [...optionsBroken, null];
106-
}
107126

108-
return [
109-
...optionsBroken,
110-
null, // [3]
111-
];
112-
113-
/*
114-
[1] @BUG RMT-518 - explanation from PR#18
115-
The "" (empty string) is to keep retrocompatibily with previous version.
116-
The "" does NOT match the JSON Schema specs. In the specs the `oneOf` keyword does not allow "" value by default.
117-
118-
Disallowing "" would be a major BREAKING CHANGE
119-
because before any string was allowed but now only the options[].value are,
120-
which means we'd need to also exclude "" from being accepted.
121-
122-
This is a dangerous change as it can disrupt existing UI Form integrations
123-
that might handle empty fields differently ("" vs null vs undefined).
124-
[2] The null needs to be allowed in required fields to show the
125-
message "Required field" instead of "The option null is not valid".
126-
[3] The nulls needs to be allowed in conventional optional fields because
127-
some consumers (eg Remote) expect empty fields to be sent as "null"
128-
even though their JSON Schemas are not created correctly (missing an option with const: null).
129-
This will allow the JSF to still mark the field as valid (false positive)
130-
and let the JSON Schema validator fail.
131-
132-
We'd need to implement a feature_flag/transition deprecation warning
133-
to give devs the time to adapt their integrations before we fix this behavior.
134-
Check the PR#18 and tests for more details.
135-
*/
127+
return isOptionalWithNull ? [...allValues, null] : allValues;
136128
};
137129

138130
const getYupSchema = ({ inputType, ...field }) => {
139131
const jsonType = getJsonTypeInArray(field.jsonType);
140132

141133
if (field.options?.length > 0) {
142-
const optionsAllowed = getOptionsAllowed(field);
143-
return yupSchemas.radioOrSelect(optionsAllowed);
134+
const optionValues = getOptions(field);
135+
return yupSchemas.radioOrSelect(optionValues);
144136
}
145137

146138
return yupSchemas[inputType] || yupSchemasToJsonTypes[jsonType];

0 commit comments

Comments
 (0)