Skip to content

Change snakecase to camelcase for prop #7460

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

Merged
merged 5 commits into from
May 12, 2025

Conversation

nojaf
Copy link
Collaborator

@nojaf nojaf commented May 10, 2025

Fixes #7451

@cknitt
Copy link
Member

cknitt commented May 11, 2025

According to the React docs, ARIA attributes should be snake case, not camel case.

See https://react.dev/reference/react-dom/components/common#common-props

In React, all ARIA attribute names are exactly the same as in HTML.

The correct JSX output should be

<label aria-label={"close sidebar"} />

So IMHO the fix should just be to remove the double quotes around the prop name.

@nojaf
Copy link
Collaborator Author

nojaf commented May 12, 2025

Interesting, I didn't know area labels were an exception here.
I'll update the code to just remove the quotes.

@zth
Copy link
Collaborator

zth commented May 12, 2025

Actually, I think the right thing is just to never escape/add quotes to the prop name. Any prop name can use -.

@nojaf
Copy link
Collaborator Author

nojaf commented May 12, 2025

The check added #7429 might not have been the best fit. I've addressed it.

Copy link

pkg-pr-new bot commented May 12, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7460

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7460

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7460

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7460

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7460

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7460

commit: be40265

Copy link
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

Looks good to me now! Thanks!

@nojaf nojaf merged commit d825723 into rescript-lang:master May 12, 2025
21 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.

JSX preserve: prop with hypen
3 participants