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-no-constructed-context-values: fix false positive for usage in non-components #3448

Merged
merged 1 commit into from Oct 3, 2022
Merged

[Fix] jsx-no-constructed-context-values: fix false positive for usage in non-components #3448

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 @@ -9,10 +9,12 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [`no-unknown-property`]: add `dialog` attributes ([#3436][] @ljharb)
* [`no-arrow-function-lifecycle`]: when converting from an arrow, remove the semi and wrapping parens ([#3337][] @ljharb)
* [`jsx-key`]: Ignore elements inside `React.Children.toArray()` ([#1591][] @silvenon)
* [`jsx-no-constructed-context-values`]: fix false positive for usage in non-components ([#3448][] @golopot)

### Changed
* [Docs] [`no-unknown-property`]: fix typo in link ([#3445][] @denkristoffer)

[#3448]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3448
[#3445]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3445
[#3436]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3436
[#3337]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3337
Expand Down
10 changes: 8 additions & 2 deletions lib/rules/jsx-no-constructed-context-values.js
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@

'use strict';

const Components = require('../util/Components');
const docsUrl = require('../util/docsUrl');
const report = require('../util/report');

Expand Down Expand Up @@ -139,7 +140,8 @@ module.exports = {
messages,
},

create(context) {
// eslint-disable-next-line arrow-body-style
create: Components.detect((context, components, utils) => {
return {
JSXOpeningElement(node) {
const openingElementName = node.name;
Expand Down Expand Up @@ -184,6 +186,10 @@ module.exports = {
return;
}

if (!utils.getParentComponent(node)) {
return;
}

// Report found error
const constructType = constructInfo.type;
const constructNode = constructInfo.node;
Expand Down Expand Up @@ -214,5 +220,5 @@ module.exports = {
});
},
};
},
}),
};
16 changes: 13 additions & 3 deletions tests/lib/rules/jsx-no-constructed-context-values.js
View file
Open in desktop
Expand Up @@ -31,13 +31,13 @@ const ruleTester = new RuleTester({ parserOptions });
ruleTester.run('react-no-constructed-context-values', rule, {
valid: parsers.all([
{
code: '<Context.Provider value={props}></Context.Provider>',
code: 'const Component = () => <Context.Provider value={props}></Context.Provider>',
},
{
code: '<Context.Provider value={100}></Context.Provider>',
code: 'const Component = () => <Context.Provider value={100}></Context.Provider>',
},
{
code: '<Context.Provider value="Some string"></Context.Provider>',
code: 'const Component = () => <Context.Provider value="Some string"></Context.Provider>',
Copy link
Member

Choose a reason for hiding this comment

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

presumably all of these existing test cases would still pass with this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I change it because otherwise these test cases would pass because they are not in components, not because other any reason which is supposed to be tested.

},
{
code: 'function Component() { const foo = useMemo(() => { return {} }, []); return (<Context.Provider value={foo}></Context.Provider>)}',
Expand Down Expand Up @@ -137,6 +137,16 @@ ruleTester.run('react-no-constructed-context-values', rule, {
}
`,
},
{
code: `
const root = ReactDOM.createRoot(document.getElementById('root'));
root.render(
<AppContext.Provider value={{}}>
<AppView />
</AppContext.Provider>
);
`,
},
]),
invalid: parsers.all([
{
Expand Down