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] jsx-sort-props: add className to RESERVED_PROPS_LIST #3851

Closed
wants to merge 1 commit into from

Conversation

saimonkat
Copy link

@saimonkat saimonkat commented Nov 15, 2024

This PR brings a new className prop to RESERVED_PROPS_LIST at jsx-sort-props rule. Related to this issue: #3175

Context
In many organizations, including ours, there is a coding standard that requires the className prop to be listed before all other props. This practice enhances code readability and consistency, making it easier for developers to quickly identify the styling-related aspects of a component.

Justification
While the current RESERVED_PROPS_LIST is designed to include only React's reserved props (children, dangerouslySetInnerHTML, key, ref), the addition of className aligns with a common industry practice that prioritizes styling props for better visibility. This change would provide flexibility for teams that adhere to this convention without impacting those who do not.

@ljharb, I completely understand the importance of maintaining the integrity of the reserved props list. However, given the widespread adoption of this convention, I believe that including className could benefit many teams. I hope you will consider this addition to support a broader range of coding standards.

Thank you for considering this request. 🙏

@ljharb
Copy link
Member

ljharb commented Nov 15, 2024

I think the problem is that it's a harmful convention - see https://medium.com/@JanPaul123/don-t-pass-css-classes-between-components-e9f7ab192785.

@saimonkat
Copy link
Author

@ljharb Thank you for your feedback. I understand your concerns about complexity, but I believe the use of className has evolved since 2015.

Today, utility-first CSS frameworks like Tailwind CSS have made className a standard for styling components efficiently. It allows for quick, consistent styling and is widely adopted in the React community.

Prioritizing className in prop sorting can enhance readability and maintainability, especially in large codebases. It aligns with modern practices where component-based styling is key.

@ljharb
Copy link
Member

ljharb commented Nov 15, 2024

To support this, the only mechanism that would make sense is a new option to add an explicit list of prop names that should appear below reserved, above "the rest", and otherwise sorted in the same way as "the rest" are within that group.

Adding a non-reserved prop to the reserved props list is just wildly incorrect.

@saimonkat
Copy link
Author

saimonkat commented Nov 15, 2024

Ok @ljharb I see. Would you mind if I update this PR with such additional props list?

As I remember it has been mentioned in a couple of more issues: #3639 #3193

@ljharb
Copy link
Member

ljharb commented Nov 15, 2024

Sure, let's do that.

@saimonkat saimonkat closed this by deleting the head repository Nov 17, 2024
@ljharb
Copy link
Member

ljharb commented Nov 17, 2024

@saimonkat by deleting the fork, this PR is now unrecoverable :-/ please don't delete forks.

@saimonkat
Copy link
Author

@saimonkat by deleting the fork, this PR is now unrecoverable :-/ please don't delete forks.

@ljharb I have misspelled the fork name and will open a new one with the properly name, referring to this thread

@ljharb
Copy link
Member

ljharb commented Nov 17, 2024

For future reference, you can rename the fork without deleting it. You basically never need to delete a repo, to fix almost anything.

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

Successfully merging this pull request may close these issues.

2 participants