Skip to content

Commit c91fd8a

Browse files
committed
Check warning/invariant calls have good messages
The `warning` implementation checked that the message is long enough to be useful. See commit f503882 for more information. It makes more sense to move this into a lint rule, and also to apply it for both `warning` and `invariant`. We can safely remove stuff from the `warning` implementation as we replace the function internally anyways. facebook#4021 (comment) @spicyj
1 parent aecc48d commit c91fd8a

File tree

3 files changed

+45
-12
lines changed

3 files changed

+45
-12
lines changed

eslint-rules/__tests__/warning-and-invariant-args-test.js

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ eslintTester.addRuleTest('eslint-rules/warning-and-invariant-args', {
5858
],
5959
},
6060
{
61-
code: "warning(true, 'foobar', 'junk argument');",
61+
code: "warning(true, 'foo is a bar under foobar', 'junk argument');",
6262
errors: [
6363
{
6464
message:
@@ -67,6 +67,37 @@ eslintTester.addRuleTest('eslint-rules/warning-and-invariant-args', {
6767
},
6868
],
6969
},
70+
{
71+
code: "invariant(true, 'error!');",
72+
errors: [
73+
{
74+
message:
75+
'The invariant format should be able to uniquely identify this ' +
76+
'invariant. Please, use a more descriptive format than: error!',
77+
},
78+
],
79+
},
80+
{
81+
code: "warning(true, 'error!');",
82+
errors: [
83+
{
84+
message:
85+
'The warning format should be able to uniquely identify this ' +
86+
'warning. Please, use a more descriptive format than: error!',
87+
},
88+
],
89+
},
90+
{
91+
code: "warning(true, '%s %s, %s %s: %s (%s)', 1, 2, 3, 4, 5, 6);",
92+
errors: [
93+
{
94+
message:
95+
'The warning format should be able to uniquely identify this ' +
96+
'warning. Please, use a more descriptive format than: ' +
97+
'%s %s, %s %s: %s (%s)',
98+
},
99+
],
100+
},
70101
],
71102
});
72103

eslint-rules/warning-and-invariant-args.js

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,26 @@ module.exports = function(context) {
4949
);
5050
return;
5151
}
52-
var str = getLiteralString(node.arguments[1]);
53-
if (str === null) {
52+
var format = getLiteralString(node.arguments[1]);
53+
if (format === null) {
5454
context.report(
5555
node,
5656
'The second argument to {{name}} must be a string literal',
5757
{name: node.callee.name}
5858
);
5959
return;
6060
}
61-
// count the number of string substitutions, plus the first two args
62-
var expectedNArgs = (str.match(/%s/g) || []).length + 2;
61+
if (format.length < 10 || /^[s\W]*$/.test(format)) {
62+
context.report(
63+
node,
64+
'The {{name}} format should be able to uniquely identify this ' +
65+
'{{name}}. Please, use a more descriptive format than: {{format}}',
66+
{name: node.callee.name, format: format}
67+
);
68+
return;
69+
}
70+
// count the number of formating substitutions, plus the first two args
71+
var expectedNArgs = (format.match(/%s/g) || []).length + 2;
6372
if (node.arguments.length !== expectedNArgs) {
6473
context.report(
6574
node,

src/shared/vendor/core/warning.js

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,6 @@ if (__DEV__) {
3131
);
3232
}
3333

34-
if (format.length < 10 || /^[s\W]*$/.test(format)) {
35-
throw new Error(
36-
'The warning format should be able to uniquely identify this ' +
37-
'warning. Please, use a more descriptive format than: ' + format
38-
);
39-
}
40-
4134
if (format.indexOf('Failed Composite propType: ') === 0) {
4235
return; // Ignore CompositeComponent proptype check.
4336
}

0 commit comments

Comments
 (0)