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.

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

New rule: jsx-prefer-fragment (#147) #161

Closed
wants to merge 1 commit into from

Conversation

banyan
Copy link

@banyan banyan commented May 6, 2018

Fixes #147.

@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint-react, @banyan! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@banyan banyan force-pushed the banyan-jsx-prefer-fragment branch from e5f56f5 to 35c11bd Compare May 7, 2018 01:00
@jkillian
Copy link
Contributor

@banyan thank you for your PR! I left a comment over here about the idea of the rule in general, let me know your thoughts

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

I'm concerned about the generalizability of this rule as well, as per #147 (comment)

I feel like you'd end up with a bunch of tslint:disable statements in random places, and it will be hard for devs to understand why some wrappers are fragments and some are divs. encoding this logic into a lint rule makes it worse because an inexperienced dev will be coerced into thinking that fragments are always better. have you tried this rule on any sizable codebase?

function render(){
return(
<div>
~~~ [An empty 'div' should be written as React.Fragment.]
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not an empty div, it's a plain div without any attributes set

@adidahiya
Copy link
Contributor

Closing due to inactivity and age. See #210

@adidahiya adidahiya closed this Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New rule suggestion: prefer-fragment
4 participants