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

feat: camelCase CSS properties in style objects #259

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ginnwork
Copy link

This PR adds the ability to use camelCase CSS property names in style objects.

Example

// old
<div style={{ 'margin-top': '40px' }}>
// new
<div style={{ marginTop: '40px' }}>

Testing

Testing has been added to ensure the Babel transform converts camelCase CSS property names to kebab-case ones.

Definitions

The JSX definitions file has also been updated and TypeScript/IntelliSense recognizes the camelCase CSS property names.

@ryansolid
Copy link
Owner

The challenge with these sort of changes is they have to be applied both at compile and at runtime to handle things like variables being passed in or spreads. So changing in compiler isn't enough. It's part of the way I have done this. I think having a single way to set stuff is better generally as it doesn't get weird conflicts say in spreads. I guess you could always convert first, but things get more complex when you have a combination of spread and non-spread. There may be a time to address this in the future but spreads make for a lot more complexity here.

@edemaine
Copy link
Contributor

edemaine commented Aug 9, 2023

I was going to suggest warnings about camel case in dev mode, but it turns out this is already a eslint-plugin-solid lint rule. There's even a --fix option to fix mistaken usage of camel case (e.g. coming from React). I imagine it only covers easy cases (not dynamic objects) but hopefully it catches most misuse.

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.

3 participants