-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix(eslint): deactivate react/no-leaked-conditional-rendering
rule
#263
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.
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
Conversation
|
🦋 Changeset detectedLatest commit: 59df482 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update to the Changes
Sequence Diagram(s)sequenceDiagram
participant Editor
participant Factory
participant ReactConfig
Editor->>Factory: Initialize ESLint config
Factory->>Factory: Detect editor environment
Factory->>Factory: console.info("Running in editor, disabling some rules")
Factory->>ReactConfig: Apply React config with ...typescriptOptions
ReactConfig-->>Factory: Return configured rules
Factory-->>Editor: Provide full ESLint config
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
Scope: all 2 workspace projects This error happened while installing a direct dependency of /tmp/eslint/packages/eslint-config Packages found in the workspace: ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/eslint-config/src/factory.ts (1)
183-185
: TypeScript options now included in React configurationThe TypeScript options are now properly spread into the React configuration, which should help provide the necessary type information for type-aware rules. This is a step toward resolving the issue with the currently disabled
react/no-leaked-conditional-rendering
rule.Once the parserOptions issue is resolved, consider re-enabling the
react/no-leaked-conditional-rendering
rule, as this spreading of TypeScript options may help provide the necessary type information.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/fluffy-emus-burn.md
(1 hunks)packages/eslint-config/src/configs/javascript.ts
(1 hunks)packages/eslint-config/src/configs/react.ts
(1 hunks)packages/eslint-config/src/factory.ts
(2 hunks)
🔇 Additional comments (4)
.changeset/fluffy-emus-burn.md (1)
1-5
: LGTM: Changeset captures the intent correctlyThe changeset correctly documents that
react/no-leaked-conditional-rendering
rule is being temporarily disabled due to aparserOptions
issue, with the intention to re-enable it once the issue is resolved.packages/eslint-config/src/configs/react.ts (1)
172-172
:✅ Verification successful
Appropriate temporary solution with clear documentation
The rule
react/no-leaked-conditional-rendering
has been properly commented out with a clear comment explaining why it's disabled (parserOptions issue). This aligns with the changeset and PR objectives.
🌐 Web query:
What are the requirements for the ESLint rule "react/no-leaked-conditional-rendering" to work properly?
💡 Result:
The ESLint rule
react/no-leaked-conditional-rendering
(also known asno-leaked-conditional-rendering
in ESLint React orjsx-no-leaked-render
in eslint-plugin-react) is designed to prevent problematic leaked values from being rendered in React components. This rule helps developers avoid unexpected rendering behavior that can occur when using the&&
operator for conditional rendering.Rule Purpose
The rule targets a common issue in React development where using the
&&
operator for conditional rendering can lead to unwanted values being displayed or even application crashes[1][2]. For example, values like0
,NaN
, or empty strings can "leak" into the rendered output when they're on the left side of an&&
expression[4].In React Native, this issue is even more severe as rendering these leaked values can crash the application entirely[1][7].
Configuration Requirements
To use this rule effectively, you need to:
Install the appropriate ESLint plugin:
- Either
eslint-plugin-react
(where the rule is calledjsx-no-leaked-render
)- Or
@eslint-react/eslint-plugin
(where it's calledno-leaked-conditional-rendering
)[1][2]Configure the rule in your ESLint config:
{ "rules": { "react/jsx-no-leaked-render": ["error", { "validStrategies": ["ternary", "coerce"] }] } }Or for the ESLint React version:
{ "rules": { "no-leaked-conditional-rendering": "error" } }Understand the rule options:
- The rule accepts a
validStrategies
option that can include"coerce"
,"ternary"
, or both (default is["ternary", "coerce"]
)[2]- The order of strategies matters for autofix behavior, with the first one being used when fixing issues[2]
Valid Strategies
The rule supports two main strategies for preventing leaked renders:
Coerce strategy: Transforms expressions by coercing values to booleans
- Uses
!!value
orBoolean(value)
to ensure boolean conversion- Example:
{!!count && <Component />}
[2][4]Ternary strategy: Uses ternary expressions instead of
&&
- Example:
{count ? <Component /> : null}
[2][4]Examples of Issues Caught by the Rule
The rule will flag code like:
// Problematic code const Component = ({ count, title }) => { return <div>{count && title}</div> }This is problematic because if
count
is0
orNaN
, it would render those values instead of nothing[2][4].Recommended Fixes
Depending on your configuration, the rule will suggest:
Using boolean coercion:
const Component = ({ count, title }) => { return <div>{!!count && title}</div> }Using ternary operators:
const Component = ({ count, title }) => { return <div>{count ? title : null}</div> }With autofix enabled, the rule can automatically transform your code to follow these patterns[2][4].
Integration with TypeScript ESLint
While this rule isn't part of the TypeScript-ESLint ecosystem directly, there have been proposals to integrate similar functionality with TypeScript's type checking to make the rule even more powerful[7]. Some developers use this rule alongside TypeScript's
strict-boolean-expressions
rule for more comprehensive protection.I hope this helps clarify how to use the
no-leaked-conditional-rendering
rule effectively in your React projects.Citations:
- 1: https://eslint-react.xyz/docs/rules/no-leaked-conditional-rendering
- 2: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/jsx-no-leaked-render.md
- 3: [New]
jsx-no-leaked-render
: addignoreAttributes
option jsx-eslint/eslint-plugin-react#3441- 4: https://dev.to/maafaishal/avoid-operator-for-conditional-rendering-in-react-2de
- 5: https://eslint-react.xyz/docs/rules/no-complex-conditional-rendering
- 6: https://dev.to/timwjames/the-best-eslint-rules-for-react-projects-30i8
- 7: Proposal: tsc backed jsx conditional rendering rule typescript-eslint/typescript-eslint#2770
- 8: https://timjames.dev/blog/the-best-eslint-rules-for-react-projects-30i8
- 9: https://www.npmjs.com/package/eslint-plugin-jsx-no-leaked-values
- 10: Docs: Mention JSX conditional rendering with strict-boolean-expressions typescript-eslint/typescript-eslint#10705
Approved: Temporary disable of
react/no-leaked-conditional-rendering
is well-documented
The comment clearly explains that this rule is disabled due to parserOptions limitations. As a reminder, this rule requires:
- Installing and enabling the correct plugin (
eslint-plugin-react
or@eslint-react/eslint-plugin
)- Enabling JSX parsing via
parserOptions.ecmaFeatures.jsx = true
(or equivalent)- Configuring the rule (e.g.
"react/jsx-no-leaked-render": ["warn", { validStrategies: ["ternary","coerce"] }]
)Once the parserOptions issue is resolved, re-enable the rule with your preferred strategy settings.
packages/eslint-config/src/configs/javascript.ts (1)
57-57
: Enhanced console logging configurationThe
no-console
rule has been updated to allowconsole.info
in addition to the previously allowedwarn
anderror
methods. This change aligns with the use ofconsole.info
in the factory.ts file.packages/eslint-config/src/factory.ts (1)
110-110
: Logging updated to use console.infoChanged from
console.log
toconsole.info
, which aligns with the updated ESLint rule configuration that now allowsconsole.info
.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @mheob/[email protected] ### Patch Changes - [#263](#263) ([@mheob](https://github.com/mheob)): deactivate `react/no-leaked-conditional-rendering` rule for now; should be added again after the `parserOptions` issue is done ## Summary by CodeRabbit - **Chores** - Updated the changelog and version number for the ESLint configuration package. - Removed a changeset related to a temporary ESLint rule deactivation. Co-authored-by: Alex Böhm <[email protected]>
This fix temporarily resolves the following issue:
Summary by CodeRabbit
console.info
for editor environment messages.console.info
in addition towarn
anderror
without triggering ESLint warnings.