Skip to content

Remove enumNames #3031

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/5.x upgrade guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ There are three new packages added in RJSF version 5:

### `@rjsf/core` BREAKING CHANGES

#### Support dropped for non-standard `enumNames` property
Copy link
Member

@heath-freenome heath-freenome Aug 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nickgros Change this to an indication of deprecation... maybe it doesn't belong under the breaking change, but still listed as something to be migrated.


`enumNames` was a non-standard JSON Schema field that RJSF supported prior to version 5 to support labels that differed from an enumeration value. This behavior can still be accomplished with `oneOf` or `anyOf` containing `const` values. For more information, see [#532](https://github.com/rjsf-team/react-jsonschema-form/issues/532).

#### Types
In version 4, RJSF exported all its types directly from `@rjsf/core`.
In version 5, only the types for the `Form` component and the `withTheme()` HOC are exported directly from `@rjsf/core`.
Expand Down
5 changes: 2 additions & 3 deletions docs/api-reference/utility-functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,8 @@ The difference between mergeSchemas and mergeObjects is that mergeSchemas only c
- GenericObjectType: The merged schema object

### optionsList()
Gets the list of options from the schema. If the schema has an enum list, then those enum values are returned.
The labels for the options will be extracted from the non-standard `enumNames` if it exists othewise will be the same as the `value`.
If the schema has a `oneOf` or `anyOf`, then the value is the list of `constant` values from the schema and the label is either the `schema.title` or the value.
Gets the list of options from the schema. If the schema has an enum list, then those enum values are returned. The labels for the options will be the same as the `value`.
If the schema has a `oneOf` or `anyOf`, then the value is the list of `const` values from the schema and the label is either the `schema.title`, if it exists, or the value.
Comment on lines +253 to +254
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nickgros Revert this, and add a note about the deprecation of enumNames, to be removed in next release


#### Parameters
- schema: RJSFSchema - The schema from which to extract the options list
Expand Down
36 changes: 16 additions & 20 deletions docs/usage/single.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,26 +63,7 @@ render((

### Custom labels for `enum` fields

This library supports a custom [`enumNames`](https://github.com/rjsf-team/react-jsonschema-form/issues/57) property for `enum` fields, which, however is not JSON-Schema compliant (see below for a compliant approach).
The `enumNames` property allows defining custom labels for each option of an `enum`:

```jsx
import validator from "@rjsf/validator-ajv6";

const schema = {
type: "number",
enum: [1, 2, 3],
enumNames: ["one", "two", "three"]
};

render((
<Form schema={schema} validator={validator} />
), document.getElementById("app"));
```

Comment on lines -66 to -82
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either restore this, or maybe keep it deleted and instead add a note about enumNames being deprecated only

#### Alternative JSON-Schema compliant approach

JSON Schema has an alternative approach to enumerations using `anyOf`; react-jsonschema-form supports it as well.
JSON Schema supports the following approaches to enumerations using `oneOf`/`anyOf`; react-jsonschema-form supports it as well.

```jsx
import validator from "@rjsf/validator-ajv6";
Expand Down Expand Up @@ -119,6 +100,21 @@ render((
), document.getElementById("app"));
```

```jsx
const schema = {
"type": "number",
"oneOf": [
{"const": 1, "title": "one"},
{"const": 2, "title": "two"},
{"const": 3, "title": "three"}
]
};

render((
<Form schema={schema} validator={validator} />
), document.getElementById("app"));
```

### Disabled attribute for `enum` fields

To disable an option, use the `ui:enumDisabled` property in the uiSchema.
Expand Down
27 changes: 26 additions & 1 deletion docs/usage/widgets.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,33 @@ Here's a list of supported alternative widgets for different JSON Schema data ty
* `select`: a select box with `true` and `false` as options;
* by default, a checkbox is used

> Note: To set the labels for a boolean field, instead of using `true` and `false` you can set `enumNames` in your schema. Note that `enumNames` belongs in your `schema`, not the `uiSchema`, and the order is always `[true, false]`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep this deleted.

> Note: To set the labels for a boolean field, instead of using `true` and `false`, your schema can use `oneOf` with `const` values for both true and false, where you can specify the custom label in the `title` field. You will also need to specify a widget in your `uiSchema`. See the following example:

schema:

```json
{
"properties": {
"booleanWithCustomLabels": {
"type": "boolean",
"oneOf": [
{"const": true, "title": "Custom label for true"},
{"const": false, "title": "Custom label for false"}
]
}
}
}
```

uiSchema:

```json
{
"booleanWithCustomLabels": {
"ui:widget": "radio" // or "select"
}
}
```
## For `string` fields

* `textarea`: a `textarea` element is used;
Expand Down
26 changes: 17 additions & 9 deletions packages/core/src/components/fields/BooleanField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
optionsList,
FieldProps,
RJSFSchemaDefinition,
EnumOptionsType,
} from "@rjsf/utils";
import isObject from "lodash/isObject";

Expand Down Expand Up @@ -35,7 +36,7 @@ function BooleanField<T = any, F = any>(props: FieldProps<T, F>) {
const { widget = "checkbox", ...options } = getUiOptions<T, F>(uiSchema);
const Widget = getWidget(schema, widget, widgets);

let enumOptions;
let enumOptions: EnumOptionsType[] | undefined;

if (Array.isArray(schema.oneOf)) {
enumOptions = optionsList({
Expand All @@ -52,14 +53,21 @@ function BooleanField<T = any, F = any>(props: FieldProps<T, F>) {
.filter((o) => o) as RJSFSchemaDefinition[], // cast away the error that typescript can't grok is fixed
});
} else {
enumOptions = optionsList({
enum: schema.enum || [true, false],
enumNames:
schema.enumNames ||
(schema.enum && schema.enum[0] === false
? ["No", "Yes"]
: ["Yes", "No"]),
});
schema.enum = schema.enum ?? [true, false];
if (
schema.enum &&
schema.enum.length === 2 &&
schema.enum.every((v) => typeof v === "boolean")
) {
enumOptions = [
{ value: schema.enum[0], label: schema.enum[0] ? "Yes" : "No" },
{ value: schema.enum[1], label: schema.enum[1] ? "Yes" : "No" },
];
Comment on lines +57 to +65
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heath-freenome instead of changing the snapshots, I opted to maintain the existing behavior of labeling boolean enums as "Yes"/"No".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep this as it is, don't use enumNames

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still need to check for user-supplied enumNames, but we can continue to not use enumNames for the [true, false] case

} else {
enumOptions = optionsList({
enum: schema.enum,
});
}
}

return (
Expand Down
34 changes: 0 additions & 34 deletions packages/core/test/BooleanField_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,23 +324,6 @@ describe("BooleanField", () => {
expect(labels).eql(["No", "Yes"]);
});

it("should support enumNames for radio widgets", () => {
const { node } = createFormComponent({
schema: {
type: "boolean",
enumNames: ["Yes", "No"],
},
formData: true,
uiSchema: { "ui:widget": "radio" },
});

const labels = [].map.call(
node.querySelectorAll(".field-radio-group label"),
(label) => label.textContent
);
expect(labels).eql(["Yes", "No"]);
});

Comment on lines -327 to -343
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we restore this, but check for console.warn() message about deprecation

it("should support oneOf titles for radio widgets", () => {
const { node } = createFormComponent({
schema: {
Expand Down Expand Up @@ -452,23 +435,6 @@ describe("BooleanField", () => {
expect(onBlur.calledWith(element.id, false)).to.be.true;
});

it("should support enumNames for select", () => {
const { node } = createFormComponent({
schema: {
type: "boolean",
enumNames: ["Yes", "No"],
},
formData: true,
uiSchema: { "ui:widget": "select" },
});

const labels = [].map.call(
node.querySelectorAll(".field option"),
(label) => label.textContent
);
expect(labels).eql(["", "Yes", "No"]);
});

Comment on lines -455 to -471
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

it("should handle a focus event with checkbox", () => {
const onFocus = sandbox.spy();
const { node } = createFormComponent({
Expand Down
8 changes: 6 additions & 2 deletions packages/core/test/FormContext_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,12 @@ describe("FormContext", () => {
type: "array",
items: {
type: "string",
enum: ["foo"],
enumNames: ["bar"],
oneOf: [
{
const: "foo",
title: "bar",
},
],
},
uniqueItems: true,
},
Expand Down
7 changes: 5 additions & 2 deletions packages/playground/src/samples/alternatives.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,11 @@ module.exports = {
blendMode: {
title: "Blend mode",
type: "string",
enum: ["screen", "multiply", "overlay"],
enumNames: ["Screen", "Multiply", "Overlay"],
Comment on lines -70 to -71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not restore this

oneOf: [
{ const: "screen", title: "Screen" },
{ const: "multiply", title: "Multiply" },
{ const: "overlay", title: "Overlay" },
],
},
},
},
Expand Down
15 changes: 14 additions & 1 deletion packages/playground/src/samples/widgets.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,20 @@ export default {
title: "Custom select widget with options",
type: "string",
enum: ["foo", "bar"],
enumNames: ["Foo", "Bar"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not restore this

},
selectWidgetOptions2: {
title: "Custom select widget with options, overriding the enum titles.",
type: "string",
oneOf: [
{
const: "foo",
title: "Foo",
},
{
const: "bar",
title: "Bar",
},
],
},
},
},
Expand Down
9 changes: 4 additions & 5 deletions packages/utils/src/optionsList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ import toConstant from "./toConstant";
import { RJSFSchema, EnumOptionsType } from "./types";

/** Gets the list of options from the schema. If the schema has an enum list, then those enum values are returned. The
* labels for the options will be extracted from the non-standard `enumNames` if it exists othewise will be the same as
* the `value`. If the schema has a `oneOf` or `anyOf`, then the value is the list of `constant` values from the schema
* and the label is either the `schema.title` or the value.
* labels for the options will be the same as the `value`. If the schema has a `oneOf` or `anyOf`, then the value is the
* list of `const` values from the schema and the label is either the `schema.title` or the value.
*
* @param schema - The schema from which to extract the options list
* @returns - The list of options from the schema
Expand All @@ -13,8 +12,8 @@ export default function optionsList(
schema: RJSFSchema
): EnumOptionsType[] | undefined {
if (schema.enum) {
return schema.enum.map((value, i) => {
const label = (schema.enumNames && schema.enumNames[i]) || String(value);
Comment on lines -16 to -17
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider restoring the old hack that I removed since I'd like to not restore the change to the type. Also add a console.warn() about enumNames being deprecated, to be removed in the next major release

return schema.enum.map((value) => {
const label = String(value);
return { label, value };
});
}
Expand Down
6 changes: 2 additions & 4 deletions packages/utils/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ export type GenericObjectType = {
};

/** Map the JSONSchema7 to our own type so that we can easily bump to JSONSchema8 at some future date and only have to
* update this one type. Also, because we are supporting the non-standard `enumNames` prop in the code, define it here.
* update this one type.
*/
export type RJSFSchema = JSONSchema7 & {
enumNames?: string[];
};
export type RJSFSchema = JSONSchema7;
Comment on lines -12 to +14
Copy link
Member

@heath-freenome heath-freenome Aug 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep this change as is... let's not make the type encourage people to use enumNames


/** Map the JSONSchema7Definition to our own type so that we can easily bump to JSONSchema8Definition at some future
* date and only have to update this one type.
Expand Down
10 changes: 0 additions & 10 deletions packages/utils/test/optionsList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,9 @@ describe("optionsList()", () => {
enum: ["Opt1", "Opt2", "Opt3"],
};

const enumNameSchema = {
...enumSchema,
enumNames: ["Option1", "Option2", "Option3"],
};
expect(optionsList(enumSchema)).toEqual(
enumSchema.enum!.map((opt) => ({ label: opt, value: opt }))
);
expect(optionsList(enumNameSchema)).toEqual(
enumNameSchema.enum!.map((opt, index) => {
const label = enumNameSchema.enumNames[index] || opt;
return { label: label, value: opt };
})
);
Comment on lines -10 to -22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh, I suppose we'll restore this to, checking for console.warn()

});
it("should generate options for a oneOf|anyOf schema", () => {
const oneOfSchema = {
Expand Down