Skip to content

Commit

Permalink
Change DEV checks from throwing to console.error
Browse files Browse the repository at this point in the history
This commit revises behavior of DEV string-coercion check functions
to use console.error instead of throwing a TypeError. There will still
be an exception thrown but it will be the being-coerced value that's
doing the throwing.

The reason to change to console.error is to pick up the component stack
to make it easier for developers to find where in their code the problem
resides.

I'm still not sure if this is the right approach. It might be better to use
console.error to get the component stack, and then to also throw in the
check functions. Another approach could be to get the stack from the
user code and throw a message that includes that stack.
  • Loading branch information
justingrant committed Sep 20, 2021
1 parent 008ca28 commit f7a0e55
Show file tree
Hide file tree
Showing 8 changed files with 182 additions and 185 deletions.
16 changes: 7 additions & 9 deletions packages/react-dom/src/__tests__/ReactDOMAttribute-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,21 +94,19 @@ describe('ReactDOM unknown attribute', () => {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod-only message');
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const test = () =>
testUnknownAttributeAssignment(new TemporalLike(), null);
expect(() =>
testUnknownAttributeAssignment(new TemporalLike(), null),
).toThrowError(
new TypeError(
__DEV__
? 'The provided `unknown` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.'
: 'prod-only message',
),
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Warning: The provided `unknown` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});

Expand Down
19 changes: 10 additions & 9 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,21 +259,20 @@ describe('ReactDOMComponent', () => {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod-only message');
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const style = {fontSize: new TemporalLike()};
const div = document.createElement('div');
expect(() => ReactDOM.render(<span style={style} />, div)).toThrowError(
new TypeError(
__DEV__
? 'The provided `fontSize` CSS property is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.'
: 'prod-only message',
),
const test = () => ReactDOM.render(<span style={style} />, div);
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Warning: The provided `fontSize` CSS property is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});

Expand Down Expand Up @@ -2440,13 +2439,15 @@ describe('ReactDOMComponent', () => {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod-only message');
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}

// `dangerouslySetInnerHTML` is never coerced to a string, so won't throw
// even with a Temporal-like object.
const container = document.createElement('div');
ReactDOM.render(
<div dangerouslySetInnerHTML={{__html: new TemporalLike()}} />,
Expand Down
56 changes: 24 additions & 32 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ describe('ReactDOMInput', () => {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod-only message');
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
Expand All @@ -560,13 +560,11 @@ describe('ReactDOMInput', () => {
<input defaultValue={new TemporalLike()} type="date" />,
container,
);
expect(test).toThrowError(
new TypeError(
__DEV__
? 'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.'
: 'prod-only message',
),
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.',
);
});

Expand All @@ -575,7 +573,7 @@ describe('ReactDOMInput', () => {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod-only message');
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
Expand All @@ -586,13 +584,11 @@ describe('ReactDOMInput', () => {
<input defaultValue={new TemporalLike()} type="text" />,
container,
);
expect(test).toThrowError(
new TypeError(
__DEV__
? 'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.'
: 'prod-only message',
),
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.',
);
});

Expand All @@ -601,7 +597,7 @@ describe('ReactDOMInput', () => {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod-only message');
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
Expand All @@ -612,13 +608,11 @@ describe('ReactDOMInput', () => {
<input value={new TemporalLike()} type="date" onChange={() => {}} />,
container,
);
expect(test).toThrowError(
new TypeError(
__DEV__
? 'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.'
: 'prod-only message',
),
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.',
);
});

Expand All @@ -627,7 +621,7 @@ describe('ReactDOMInput', () => {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod-only message');
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
Expand All @@ -638,13 +632,11 @@ describe('ReactDOMInput', () => {
<input value={new TemporalLike()} type="text" onChange={() => {}} />,
container,
);
expect(test).toThrowError(
new TypeError(
__DEV__
? 'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.'
: 'prod-only message',
),
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.',
);
});

Expand Down
Loading

0 comments on commit f7a0e55

Please sign in to comment.