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

Support for attribute #977

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Support for attribute #977

merged 1 commit into from
Sep 4, 2024

Conversation

edoardocavazza
Copy link
Contributor

This PR introduces support for for attribute as equivalent for the htmlFor property.

Related issues: #894, #961

@ljharb
Copy link
Member

ljharb commented Apr 8, 2024

This must not be done by default, since that's not how react (the original, default, and most common usage of jsx) works.

I think a global setting should be used to map htmlFor to for, and rules updated to respect that setting - that way, custom renderers can map whatever attributes they want.

@edoardocavazza
Copy link
Contributor Author

In most of the rendering engine, both for (as attribute) and htmlFor (as property) are valid. Maybe the global setting should be an array of property names to check?

@ljharb
Copy link
Member

ljharb commented Apr 9, 2024

React's is the primary one that matters since it has almost the entirety of the usage, but it's good to design the schema so it can support all the renderers.

how about an object, which defaults to { for: ['htmlFor'], 'class': ['className'] } etc, and then you can set it to any combination you want?

@edoardocavazza
Copy link
Contributor Author

edoardocavazza commented Apr 12, 2024

I think it would fit a lot, but I have some issues creating the test cases for the settings :(

@ljharb
Copy link
Member

ljharb commented Apr 15, 2024

I’d prefer to reuse this PR (but now that you’ve opened a second one, please keep both open)

@ljharb ljharb reopened this Apr 15, 2024
@ljharb ljharb merged commit a1ee7f8 into jsx-eslint:main Sep 4, 2024
208 checks passed
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.

2 participants