From 11bc26841489382c9d43dfca3e7668f984605cde Mon Sep 17 00:00:00 2001 From: igorbrasileiro Date: Sat, 21 Mar 2020 23:59:05 -0300 Subject: [PATCH 1/2] add a warning to useAutocomplete when the value doesn't exist in options --- .../src/Autocomplete/Autocomplete.test.js | 25 +++++++++++++++++++ .../src/useAutocomplete/useAutocomplete.js | 25 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js index e60baa86d3138a..6c569b95da4255 100644 --- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js +++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js @@ -775,12 +775,18 @@ describe('', () => { }); describe('warnings', () => { + let consoleWarnContainer = null; + beforeEach(() => { consoleErrorMock.spy(); + consoleWarnContainer = console.warn; + console.warn = spy(); }); afterEach(() => { consoleErrorMock.reset(); + console.warn = consoleWarnContainer; + consoleWarnContainer = null; }); it('warn if getOptionLabel do not return a string', () => { @@ -836,6 +842,25 @@ describe('', () => { 'The component expects a single value to match a given option but found 2 matches.', ); }); + + it('warn if value does not exist in options list', () => { + const value = 'not a good value'; + const options = ['first option', 'second option']; + + render( + } + />, + ); + + expect(console.warn.callCount).to.equal(4); + expect(console.warn.args[0][0]).to.include( + 'Material-UI: you have provided an out-of-range value `"not a good value"` for the autocomplete component.', + ); + }); }); describe('prop: options', () => { diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js index dc02585540c24f..afd29056edad3b 100644 --- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js +++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js @@ -256,6 +256,31 @@ export default function useAutocomplete(props) { popupOpen = freeSolo && filteredOptions.length === 0 ? false : popupOpen; + if (process.env.NODE_ENV !== 'production') { + if (value !== null && !freeSolo && options.length > 0) { + const missingValue = (multiple ? value : [value]).filter( + (value2) => !options.some((option) => getOptionSelected(option, value2)), + ); + + if (missingValue.length > 0) { + console.warn( + [ + `Material-UI: you have provided an out-of-range value${ + missingValue.length > 1 ? 's' : '' + } \`${ + missingValue.length > 1 + ? JSON.stringify(missingValue) + : JSON.stringify(missingValue[0]) + }\` for the autocomplete ${id ? `(id="${id}") ` : ''}component.`, + '', + 'Consider providing a value that matches one of the available options.', + 'You can use the `getOptionSelected` prop to customize the equality test.', + ].join('\n'), + ); + } + } + } + const focusTag = useEventCallback((tagToFocus) => { if (tagToFocus === -1) { inputRef.current.focus(); From f17d5c6fbefcf5e75a9b9995a7a869149061cb90 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sun, 22 Mar 2020 20:56:05 +0100 Subject: [PATCH 2/2] normalize a bit more the error messages --- .../src/Autocomplete/Autocomplete.test.js | 16 +++++-------- .../src/useAutocomplete/useAutocomplete.js | 16 +++++-------- packages/material-ui/src/Tabs/Tabs.js | 4 ++-- test/utils/consoleErrorMock.js | 23 ++++++++++++------- 4 files changed, 29 insertions(+), 30 deletions(-) diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js index 6c569b95da4255..282f987815b3b3 100644 --- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js +++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js @@ -2,7 +2,7 @@ import * as React from 'react'; import { expect } from 'chai'; import { createMount, getClasses } from '@material-ui/core/test-utils'; import describeConformance from '@material-ui/core/test-utils/describeConformance'; -import consoleErrorMock from 'test/utils/consoleErrorMock'; +import consoleErrorMock, { consoleWarnMock } from 'test/utils/consoleErrorMock'; import { spy } from 'sinon'; import { createClientRender, fireEvent } from 'test/utils/createClientRender'; import { createFilterOptions } from '../useAutocomplete/useAutocomplete'; @@ -775,18 +775,14 @@ describe('', () => { }); describe('warnings', () => { - let consoleWarnContainer = null; - beforeEach(() => { consoleErrorMock.spy(); - consoleWarnContainer = console.warn; - console.warn = spy(); + consoleWarnMock.spy(); }); afterEach(() => { consoleErrorMock.reset(); - console.warn = consoleWarnContainer; - consoleWarnContainer = null; + consoleWarnMock.reset(); }); it('warn if getOptionLabel do not return a string', () => { @@ -856,9 +852,9 @@ describe('', () => { />, ); - expect(console.warn.callCount).to.equal(4); - expect(console.warn.args[0][0]).to.include( - 'Material-UI: you have provided an out-of-range value `"not a good value"` for the autocomplete component.', + expect(consoleWarnMock.callCount()).to.equal(4); + expect(consoleWarnMock.messages()[0]).to.include( + 'None of the options match with `"not a good value"`', ); }); }); diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js index afd29056edad3b..35a0feb61f4946 100644 --- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js +++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js @@ -200,10 +200,9 @@ export default function useAutocomplete(props) { const erroneousReturn = optionLabel === undefined ? 'undefined' : `${typeof optionLabel} (${optionLabel})`; console.error( - [ - `Material-UI: the \`getOptionLabel\` method of ${componentName} returned ${erroneousReturn} instead of a string for`, - JSON.stringify(newValue), - ].join('\n'), + `Material-UI: the \`getOptionLabel\` method of ${componentName} returned ${erroneousReturn} instead of a string for ${JSON.stringify( + newValue, + )}.`, ); } } @@ -265,15 +264,12 @@ export default function useAutocomplete(props) { if (missingValue.length > 0) { console.warn( [ - `Material-UI: you have provided an out-of-range value${ - missingValue.length > 1 ? 's' : '' - } \`${ + `Material-UI: the value provided to ${componentName} is invalid.`, + `None of the options match with \`${ missingValue.length > 1 ? JSON.stringify(missingValue) : JSON.stringify(missingValue[0]) - }\` for the autocomplete ${id ? `(id="${id}") ` : ''}component.`, - '', - 'Consider providing a value that matches one of the available options.', + }\`.`, 'You can use the `getOptionSelected` prop to customize the equality test.', ].join('\n'), ); diff --git a/packages/material-ui/src/Tabs/Tabs.js b/packages/material-ui/src/Tabs/Tabs.js index dc23fcd2bcf7aa..b59620fb19886b 100644 --- a/packages/material-ui/src/Tabs/Tabs.js +++ b/packages/material-ui/src/Tabs/Tabs.js @@ -153,8 +153,8 @@ const Tabs = React.forwardRef(function Tabs(props, ref) { if (!tab) { console.error( [ - `Material-UI: the value provided \`${value}\` to the Tabs component is invalid.`, - 'None of the Tabs children have this value.', + `Material-UI: the value provided to the Tabs component is invalid.`, + `None of the Tabs' children match with \`${value}\`.`, valueToIndex.keys ? `You can provide one of the following values: ${Array.from( valueToIndex.keys(), diff --git a/test/utils/consoleErrorMock.js b/test/utils/consoleErrorMock.js index 928e3119fc4494..fb3e92f287c056 100644 --- a/test/utils/consoleErrorMock.js +++ b/test/utils/consoleErrorMock.js @@ -1,3 +1,4 @@ +/* eslint-disable no-console */ import utilFormat from 'format-util'; import { spy } from 'sinon'; @@ -14,22 +15,26 @@ import { spy } from 'sinon'; * warning.restore(); * }); */ -class ConsoleErrorMock { +export class ConsoleMock { consoleErrorContainer; + constructor(methodName) { + this.methodName = methodName; + } + spy = () => { - this.consoleErrorContainer = console.error; - console.error = spy(); + this.consoleErrorContainer = console[this.methodName]; + console[this.methodName] = spy(); }; reset = () => { - console.error = this.consoleErrorContainer; + console[this.methodName] = this.consoleErrorContainer; delete this.consoleErrorContainer; }; callCount = () => { if (this.consoleErrorContainer) { - return console.error.callCount; + return console[this.methodName].callCount; } throw new Error('Requested call count before spy() was called'); @@ -45,7 +50,7 @@ class ConsoleErrorMock { /** * returns the formatted message for each call * - * you could call console.error("type %s", "foo") which would log + * you could call console[this.methodName]("type %s", "foo") which would log * "type foo". If you want to assert on the actual message use messages() instead */ messages = () => { @@ -56,11 +61,13 @@ class ConsoleErrorMock { /** * @type {import('sinon').SinonSpy} */ - const consoleSpy = console.error; + const consoleSpy = console[this.methodName]; return consoleSpy.args.map((loggerArgs) => { return utilFormat(...loggerArgs); }); }; } -export default new ConsoleErrorMock(); +export const consoleWarnMock = new ConsoleMock('warn'); + +export default new ConsoleMock('error');