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

[Feat] `no-render-return-undefined`: disallow components rendering undefined #3750

Draft

akulsr0 wants to merge 9 commits into jsx-eslint:master from akulsr0:akul/no-render-return-undefined

+1,149 −0

Contributor

akulsr0 commented May 8, 2024 •

edited

Closes #3020


          [Feat] no-render-return-undefined: initital commit

499e261

codecov bot commented May 8, 2024 •

edited

Codecov Report

Attention: Patch coverage is 85.83333% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 51.86%. Comparing base (8e1a94b) to head (6d3cfec).
Report is 7 commits behind head on master.

❗ Current head 6d3cfec differs from pull request most recent head d1814c1

Please upload reports for the commit d1814c1 to get more accurate results.

Files	Patch %	Lines
lib/util/eslint.js	47.82%	12 Missing ⚠️
lib/util/ast.js	55.55%	4 Missing ⚠️
lib/util/variable.js	80.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #3750       +/-   ##
===========================================
- Coverage   97.62%   51.86%   -45.77%     
===========================================
  Files         134        8      -126     
  Lines        9605      349     -9256     
  Branches     3486      128     -3358     
===========================================
- Hits         9377      181     -9196     
+ Misses        228      168       -60

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

akulsr0 added 4 commits

May 8, 2024 14:27


          chore: add more test cases

86d7ac8


          chore: add test cases

61981c7


          chore: add tc for class components

34b6a94


          chore: handle returning call expressions

56e7436

akulsr0 marked this pull request as ready for review

May 15, 2024 10:35


          chore: handle returning logical expressions

9202a30

ljharb marked this pull request as draft

May 15, 2024 17:23


          docs: add rule documentation

639f15f

Contributor Author

akulsr0 commented May 16, 2024

@ljharb Let me know your thoughts on this!


          chore: handle map callbacks

8aec646

akulsr0 marked this pull request as ready for review

May 18, 2024 14:50

ljharb added the new rule label

ljharb requested changes

View reviewed changes

docs/rules/no-render-return-undefined.md

    
            @@ -0,0 +1,62 @@
          
              # Disallow returning undefined from react components (`react/no-render-return-undefined`)

              💼 This rule is enabled in the ☑️ `recommended` [config](https://github.com/jsx-eslint/eslint-plugin-react/#shareable-configs).

Member

ljharb May 24, 2024

i think the docs need to be updated, run npm run update:eslint-docs

docs/rules/no-render-return-undefined.md Outdated

    
              <!-- end auto-generated rule header -->

              > In React 18, components may render undefined, and React will render nothing to the DOM instead of throwing an error. However, accidentally rendering nothing in a component could still cause surprises. This rule will warn if the `return` statement in a React Component returns undefined.

Member

ljharb May 24, 2024

Suggested change

      
            > In React 18, components may render undefined, and React will render nothing to the DOM instead of throwing an error. However, accidentally rendering nothing in a component could still cause surprises. This rule will warn if the `return` statement in a React Component returns undefined.
          
            > Starting in React 18, components may render `undefined`, and React will render nothing to the DOM instead of throwing an error. However, accidentally rendering nothing in a component could still cause surprises. This rule will warn if the `return` statement in a React Component returns `undefined`.

docs/rules/no-render-return-undefined.md Outdated

    
              > In React 18, components may render undefined, and React will render nothing to the DOM instead of throwing an error. However, accidentally rendering nothing in a component could still cause surprises. This rule will warn if the `return` statement in a React Component returns undefined.

              Issue: [#3020](https://github.com/jsx-eslint/eslint-plugin-react/issues/3020)

Member

ljharb May 24, 2024

i'm not sure linking to the issue is needed

docs/rules/no-render-return-undefined.md Outdated

    
              ## Rule Details

              This rule will warn if the `return` statement in a React component returns undefined.

Member

ljharb May 24, 2024

Suggested change

      
            This rule will warn if the `return` statement in a React component returns undefined.
          
            This rule will warn if the `return` statement in a React component returns `undefined`.

docs/rules/no-render-return-undefined.md

    
              This rule will warn if the `return` statement in a React component returns undefined.

              Examples of **incorrect** code for this rule:

Member

ljharb May 24, 2024

let's add one that uses void

lib/rules/no-render-return-undefined.js Outdated

    
                },

                create(context) {

                  function getReturnValue(returnNode) {

Member

ljharb May 24, 2024

can this take context and be defined at module level instead?

lib/rules/no-render-return-undefined.js Outdated

    
                      }

                      case 'ConditionalExpression': {

                        const possibleReturnValue = [getReturnValue(returnNode.consequent), getReturnValue(returnNode.alternate)];

                        const returnsUndefined = possibleReturnValue.some((val) => val === undefined);

Member

ljharb May 24, 2024

Suggested change

      
                      const returnsUndefined = possibleReturnValue.some((val) => val === undefined);
          
                      const returnsUndefined = possibleReturnValue.some((val) => typeof val === 'undefined');

lib/rules/no-render-return-undefined.js Outdated

    
                      case 'ConditionalExpression': {

                        const possibleReturnValue = [getReturnValue(returnNode.consequent), getReturnValue(returnNode.alternate)];

                        const returnsUndefined = possibleReturnValue.some((val) => val === undefined);

                        if (returnsUndefined) return undefined;

Member

ljharb May 24, 2024

Suggested change

      
                      if (returnsUndefined) return undefined;
          
                      if (returnsUndefined) return;

lib/rules/no-render-return-undefined.js Outdated

    
                    return !returnStatement

                        || returnIdentifierName === 'undefined'

                        || returnIdentifierValue === undefined

Member

ljharb May 24, 2024

Suggested change

      
                      || returnIdentifierValue === undefined
          
                      || typeof returnIdentifierValue === 'undefined'

package.json Outdated

    
            @@ -15,7 +15,7 @@
          
                  "test": "npm run unit-test",

                  "posttest": "aud --production",

                  "type-check": "tsc",

                  "unit-test": "istanbul cover node_modules/mocha/bin/_mocha tests/lib/**/*.js tests/util/**/*.js tests/index.js",

                  "unit-test": "istanbul cover node_modules/mocha/bin/_mocha tests/lib/rules/no-render-return-undefined.js",

Member

ljharb May 24, 2024

commenting here to ensure this doesn't get merged. fwiw you can npx mocha tests/lib/rules/no-render-return-undefined.js to run just that one file locally :-)

ljharb marked this pull request as draft

May 24, 2024 16:34


          refactor: pr review changes

d1814c1

akulsr0 force-pushed the akul/no-render-return-undefined branch from 6d3cfec to d1814c1 Compare

May 25, 2024 05:13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment