Skip to content
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

[Autocomplete] Warn when value does not match options #20235

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
23 changes: 22 additions & 1 deletion packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -777,10 +777,12 @@ describe('<Autocomplete />', () => {
describe('warnings', () => {
beforeEach(() => {
consoleErrorMock.spy();
consoleWarnMock.spy();
});

afterEach(() => {
consoleErrorMock.reset();
consoleWarnMock.reset();
});

it('warn if getOptionLabel do not return a string', () => {
Expand Down Expand Up @@ -836,6 +838,25 @@ describe('<Autocomplete />', () => {
'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(
<Autocomplete
{...defaultProps}
value={value}
options={options}
renderInput={(params) => <TextField {...params} />}
/>,
);

expect(consoleWarnMock.callCount()).to.equal(4);
expect(consoleWarnMock.messages()[0]).to.include(
'None of the options match with `"not a good value"`',
);
});
});

describe('prop: options', () => {
Expand Down
29 changes: 25 additions & 4 deletions packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)}.`,
);
}
}
Expand Down Expand Up @@ -256,6 +255,28 @@ 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: the value provided to ${componentName} is invalid.`,
`None of the options match with \`${
missingValue.length > 1
? JSON.stringify(missingValue)
: JSON.stringify(missingValue[0])
}\`.`,
'You can use the `getOptionSelected` prop to customize the equality test.',
].join('\n'),
);
}
}
}

const focusTag = useEventCallback((tagToFocus) => {
if (tagToFocus === -1) {
inputRef.current.focus();
Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/src/Tabs/Tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
23 changes: 15 additions & 8 deletions test/utils/consoleErrorMock.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
import utilFormat from 'format-util';
import { spy } from 'sinon';

Expand All @@ -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');
Expand All @@ -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 = () => {
Expand All @@ -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');
Copy link
Member

@oliviertassinari oliviertassinari Mar 22, 2020

Choose a reason for hiding this comment

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

A reminder that we need to advance #15343.


export default new ConsoleMock('error');