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.

Sign up for GitHub

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

Jump to bottom

[Fix] jsx-boolean-value: make error messages clearer #3691

Merged
1 commit merged into from Mar 4, 2024
Merged

[Fix] jsx-boolean-value: make error messages clearer #3691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -30,6 +30,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [`forbid-elements`]: prevent a crash on `createElement()` ([#3632][] @ljharb)

### Changed
* [`jsx-boolean-value`]: make error messages clearer ([#3691][] @developer-bandi)
* [Refactor] `propTypes`: extract type params to var ([#3634][] @HenryBrown0)
* [Refactor] [`boolean-prop-naming`]: invert if statement ([#3634][] @HenryBrown0)
* [Refactor] [`function-component-definition`]: exit early if no type params ([#3634][] @HenryBrown0)
Expand All @@ -40,6 +41,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [Docs] [`hook-use-state`]: fix an undefined variable ([#3626][] @chentsulin)

[#3697]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3697
[#3691]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3691
[#3690]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3690
[#3680]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3680
[#3679]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3679
Expand Down
37 changes: 9 additions & 28 deletions lib/rules/jsx-boolean-value.js
View file
Open in desktop
Expand Up @@ -21,19 +21,6 @@ const exceptionsSchema = {
const ALWAYS = 'always';
const NEVER = 'never';

const errorData = new WeakMap();
/**
* @param {object} exceptions
* @returns {object}
*/
function getErrorData(exceptions) {
if (!errorData.has(exceptions)) {
const exceptionProps = Array.from(exceptions, (name) => `\`${name}\``).join(', ');
const exceptionsMessage = exceptions.size > 0 ? ` for the following props: ${exceptionProps}` : '';
errorData.set(exceptions, { exceptionsMessage });
}
return errorData.get(exceptions);
}
/**
* @param {string} configuration
* @param {Set<string>} exceptions
Expand Down Expand Up @@ -62,12 +49,9 @@ function isNever(configuration, exceptions, propName) {
}

const messages = {
omitBoolean: 'Value must be omitted for boolean attributes{{exceptionsMessage}}',
omitBoolean_noMessage: 'Value must be omitted for boolean attributes',
setBoolean: 'Value must be set for boolean attributes{{exceptionsMessage}}',
setBoolean_noMessage: 'Value must be set for boolean attributes',
omitPropAndBoolean: 'Value and Prop must be omitted for false attributes{{exceptionsMessage}}',
omitPropAndBoolean_noMessage: 'Value and Prop must be omitted for false attributes',
omitBoolean: 'Value must be omitted for boolean attribute `{{propName}}`',
setBoolean: 'Value must be set for boolean attribute `{{propName}}`',
omitPropAndBoolean: 'Value must be omitted for `false` attribute: `{{propName}}`',
};

module.exports = {
Expand Down Expand Up @@ -135,8 +119,8 @@ module.exports = {
isAlways(configuration, exceptions, propName)
&& value === null
) {
const data = getErrorData(exceptions);
const messageId = data.exceptionsMessage ? 'setBoolean' : 'setBoolean_noMessage';
const messageId = 'setBoolean';
const data = { propName };
report(context, messages[messageId], messageId, {
node,
data,
Expand All @@ -152,8 +136,8 @@ module.exports = {
&& value.type === 'JSXExpressionContainer'
&& value.expression.value === true
) {
const data = getErrorData(exceptions);
const messageId = data.exceptionsMessage ? 'omitBoolean' : 'omitBoolean_noMessage';
const messageId = 'omitBoolean';
const data = { propName };
report(context, messages[messageId], messageId, {
node,
data,
Expand All @@ -169,11 +153,8 @@ module.exports = {
&& value.type === 'JSXExpressionContainer'
&& value.expression.value === false
) {
const data = getErrorData(exceptions);
const messageId = data.exceptionsMessage
? 'omitPropAndBoolean'
: 'omitPropAndBoolean_noMessage';

const messageId = 'omitPropAndBoolean';
const data = { propName };
report(context, messages[messageId], messageId, {
node,
data,
Expand Down
61 changes: 49 additions & 12 deletions tests/lib/rules/jsx-boolean-value.js
View file
Open in desktop
Expand Up @@ -63,7 +63,10 @@ ruleTester.run('jsx-boolean-value', rule, {
output: '<App foo />;',
options: ['never'],
errors: [
{ messageId: 'omitBoolean_noMessage' },
{
messageId: 'omitBoolean',
data: { propName: 'foo' },
},
],
},
{
Expand All @@ -73,34 +76,43 @@ ruleTester.run('jsx-boolean-value', rule, {
errors: [
{
messageId: 'omitBoolean',
data: { exceptionsMessage: ' for the following props: `foo`, `bar`' },
data: { propName: 'foo' },
},
{
messageId: 'omitBoolean',
data: { exceptionsMessage: ' for the following props: `foo`, `bar`' },
data: { propName: 'bar' },
},
],
},
{
code: '<App foo={true} />;',
output: '<App foo />;',
errors: [
{ messageId: 'omitBoolean_noMessage' },
{
messageId: 'omitBoolean',
data: { propName: 'foo' },
},
],
},
{
code: '<App foo = {true} />;',
output: '<App foo />;',
errors: [
{ messageId: 'omitBoolean_noMessage' },
{
messageId: 'omitBoolean',
data: { propName: 'foo' },
},
],
},
{
code: '<App foo />;',
output: '<App foo={true} />;',
options: ['always'],
errors: [
{ messageId: 'setBoolean_noMessage' },
{
messageId: 'setBoolean',
data: { propName: 'foo' },
},
],
},
{
Expand All @@ -110,11 +122,11 @@ ruleTester.run('jsx-boolean-value', rule, {
errors: [
{
messageId: 'setBoolean',
data: { exceptionsMessage: ' for the following props: `foo`, `bar`' },
data: { propName: 'foo' },
},
{
messageId: 'setBoolean',
data: { exceptionsMessage: ' for the following props: `foo`, `bar`' },
data: { propName: 'bar' },
},
],
},
Expand All @@ -123,8 +135,14 @@ ruleTester.run('jsx-boolean-value', rule, {
output: '<App />;',
options: ['never', { assumeUndefinedIsFalse: true }],
errors: [
{ messageId: 'omitPropAndBoolean_noMessage' },
{ messageId: 'omitPropAndBoolean_noMessage' },
{
messageId: 'omitPropAndBoolean',
data: { propName: 'foo' },
},
{
messageId: 'omitPropAndBoolean',
data: { propName: 'bak' },
},
],
},
{
Expand All @@ -137,11 +155,30 @@ ruleTester.run('jsx-boolean-value', rule, {
errors: [
{
messageId: 'omitPropAndBoolean',
data: { exceptionsMessage: ' for the following props: `baz`, `bak`' },
data: { propName: 'baz' },
},
{
messageId: 'omitPropAndBoolean',
data: { exceptionsMessage: ' for the following props: `baz`, `bak`' },
data: { propName: 'bak' },
},
],
},
{
code: '<App foo={true} bar={true} baz />;',
output: '<App foo bar baz={true} />;',
options: ['always', { never: ['foo', 'bar'] }],
errors: [
{
messageId: 'omitBoolean',
data: { propName: 'foo' },
},
{
messageId: 'omitBoolean',
data: { propName: 'bar' },
},
{
messageId: 'setBoolean',
data: { propName: 'baz' },
},
],
},
Expand Down