1
0
mirror of https://github.com/lensapp/lens.git synced 2025-05-20 05:10:56 +00:00

Require id for <Select /> to prevent non-deterministic renders

This was caused by global state in a 3rd party lib: "react-select".

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>
This commit is contained in:
Iku-turso 2022-03-21 16:27:21 +02:00 committed by Janne Savolainen
parent b947568b14
commit 9c024b98d3
No known key found for this signature in database
GPG Key ID: 8C6CFB2FFFE8F68A
2 changed files with 28 additions and 14 deletions

View File

@ -63,7 +63,7 @@ describe("<Select />", () => {
const onChange = jest.fn(); const onChange = jest.fn();
const { container } = render(<Select onChange={onChange} options={options} />); const { container } = render(<Select id="some-test-input" onChange={onChange} options={options} />);
expect(container).toBeInstanceOf(HTMLElement); expect(container).toBeInstanceOf(HTMLElement);
}); });
@ -82,7 +82,7 @@ describe("<Select />", () => {
const onChange = jest.fn(); const onChange = jest.fn();
const { container } = render(<Select value={options[0].value} onChange={onChange} options={options} />); const { container } = render(<Select id="some-test-input" value={options[0].value} onChange={onChange} options={options} />);
const selectedValueContainer = container.querySelector(".Select__single-value"); const selectedValueContainer = container.querySelector(".Select__single-value");
expect(selectedValueContainer.textContent).toBe(options[0].label); expect(selectedValueContainer.textContent).toBe(options[0].label);
@ -102,12 +102,12 @@ describe("<Select />", () => {
const onChange = jest.fn(); const onChange = jest.fn();
const { container, rerender } = render(<Select value={options[0].value} onChange={onChange} options={options} />); const { container, rerender } = render(<Select id="some-test-input" value={options[0].value} onChange={onChange} options={options} />);
const selectedValueContainer = container.querySelector(".Select__single-value"); const selectedValueContainer = container.querySelector(".Select__single-value");
expect(selectedValueContainer.textContent).toBe(options[0].label); expect(selectedValueContainer.textContent).toBe(options[0].label);
rerender(<Select value={options[1].value} onChange={onChange} options={options} />); rerender(<Select id="some-test-input" value={options[1].value} onChange={onChange} options={options} />);
expect(container.querySelector(".Select__single-value").textContent).toBe(options[1].label); expect(container.querySelector(".Select__single-value").textContent).toBe(options[1].label);
}); });
@ -126,12 +126,12 @@ describe("<Select />", () => {
const onChange = jest.fn(); const onChange = jest.fn();
const { container, rerender } = render(<Select value={options[0].value} onChange={onChange} options={options} />); const { container, rerender } = render(<Select id="some-test-input" value={options[0].value} onChange={onChange} options={options} />);
const selectedValueContainer = container.querySelector(".Select__single-value"); const selectedValueContainer = container.querySelector(".Select__single-value");
expect(selectedValueContainer.textContent).toBe(options[0].label); expect(selectedValueContainer.textContent).toBe(options[0].label);
rerender(<Select value={null} onChange={onChange} options={options} />); rerender(<Select id="some-test-input" value={null} onChange={onChange} options={options} />);
expect(container.querySelector(".Select__single-value")).not.toBeInTheDocument(); expect(container.querySelector(".Select__single-value")).not.toBeInTheDocument();
}); });
@ -150,12 +150,12 @@ describe("<Select />", () => {
const onChange = jest.fn(); const onChange = jest.fn();
const { container, rerender } = render(<Select value={options[0].value} onChange={onChange} options={options} />); const { container, rerender } = render(<Select id="some-test-input" value={options[0].value} onChange={onChange} options={options} />);
const selectedValueContainer = container.querySelector(".Select__single-value"); const selectedValueContainer = container.querySelector(".Select__single-value");
expect(selectedValueContainer.textContent).toBe(options[0].label); expect(selectedValueContainer.textContent).toBe(options[0].label);
rerender(<Select value={undefined} onChange={onChange} options={options} />); rerender(<Select id="some-test-input" value={undefined} onChange={onChange} options={options} />);
expect(container.querySelector(".Select__single-value")).not.toBeInTheDocument(); expect(container.querySelector(".Select__single-value")).not.toBeInTheDocument();
}); });

View File

@ -30,7 +30,10 @@ export interface SelectOption<T = any> {
label?: React.ReactNode; label?: React.ReactNode;
} }
export interface SelectProps<T = any> extends ReactSelectProps<T, boolean>, CreatableProps<T, boolean> { export interface SelectProps<T = any>
extends ReactSelectProps<T, boolean>,
CreatableProps<T, boolean> {
id: string;
value?: T; value?: T;
themeName?: "dark" | "light" | "outlined" | "lens"; themeName?: "dark" | "light" | "outlined" | "lens";
menuClass?: string; menuClass?: string;
@ -41,7 +44,7 @@ export interface SelectProps<T = any> extends ReactSelectProps<T, boolean>, Crea
@observer @observer
export class Select extends React.Component<SelectProps> { export class Select extends React.Component<SelectProps> {
static defaultProps: SelectProps = { static defaultProps: Omit<SelectProps, "id"> = {
autoConvertOptions: true, autoConvertOptions: true,
menuPortalTarget: document.body, menuPortalTarget: document.body,
menuPlacement: "auto", menuPlacement: "auto",
@ -53,7 +56,8 @@ export class Select extends React.Component<SelectProps> {
} }
@computed get themeClass() { @computed get themeClass() {
const themeName = this.props.themeName || ThemeStore.getInstance().activeTheme.type; const themeName =
this.props.themeName || ThemeStore.getInstance().activeTheme.type;
return `theme-${themeName}`; return `theme-${themeName}`;
} }
@ -88,7 +92,9 @@ export class Select extends React.Component<SelectProps> {
if (autoConvertOptions && Array.isArray(options)) { if (autoConvertOptions && Array.isArray(options)) {
return options.map(opt => { return options.map(opt => {
return this.isValidOption(opt) ? opt : { value: opt, label: String(opt) }; return this.isValidOption(opt)
? opt
: { value: opt, label: String(opt) };
}); });
} }
@ -114,13 +120,21 @@ export class Select extends React.Component<SelectProps> {
render() { render() {
const { const {
className, menuClass, isCreatable, autoConvertOptions, className,
value, options, components = {}, ...props menuClass,
isCreatable,
autoConvertOptions,
value,
options,
components = {},
id: inputId,
...props
} = this.props; } = this.props;
const WrappedMenu = components.Menu ?? Menu; const WrappedMenu = components.Menu ?? Menu;
const selectProps: Partial<SelectProps> = { const selectProps: Partial<SelectProps> = {
...props, ...props,
inputId,
styles: this.styles, styles: this.styles,
value: autoConvertOptions ? this.selectedOption : value, value: autoConvertOptions ? this.selectedOption : value,
options: autoConvertOptions ? this.options : options, options: autoConvertOptions ? this.options : options,