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

[react-jss] - when generating classes, it does not handle spaces correctly #938

Closed
bhupinderbola opened this issue Dec 14, 2018 · 9 comments
Labels
bug It went crazy and killed everyone. complexity:low You can fix it, c'mon!

Comments

@bhupinderbola
Copy link

Expected behavior:
When Component's displayName has a space, then injected styles should apply correctly.

Describe the bug:
When Component's displayName has a space, then injected styles do not apply correctly.

Codesandbox link:
https://codesandbox.io/s/4jpxqn76ow

Versions (please complete the following information):

  • react-jss: 8.6.1
  • Browser [e.g. chrome, safari]: Chrome
  • OS [e.g. Windows, macOS]: Windows
    Feel free to add any additional versions which you may think are relevant to the bug.
@kof kof added the bug It went crazy and killed everyone. label Dec 14, 2018
@kof
Copy link
Member

kof commented Dec 15, 2018

Its a bug. JSS core expects that classNamePrefix is a valid class name, react-jss uses the displayName with assumption it is an alternative to class component name or function name, which would be valid as well (https://reactjs.org/docs/react-component.html#displayname) ... so its kinda an edge case that some folks use it with arbitary strings with spaces.

To fix this we would need to export the escaping function from the core and use it in react-jss.

@kof kof added the complexity:low You can fix it, c'mon! label Dec 15, 2018
@kof
Copy link
Member

kof commented Dec 15, 2018

Also it won't be a problem in production, since we don't pass it. So we should only escape it in dev mode as well.

@kof kof closed this as completed in 24f158b Dec 24, 2018
@kof
Copy link
Member

kof commented Dec 24, 2018

Fixed in master, will be released in V10

@bhupinderbola
Copy link
Author

@kof I updated https://codesandbox.io/s/4jpxqn76ow to start using v10 but issue still exists.

@HenriBeck HenriBeck reopened this Feb 9, 2019
@HenriBeck
Copy link
Member

The identifier is correctly being escaped, the rule just isn't correctly applied. It is not possible to apply css rules which have a space inside.

See: https://cs50.stackexchange.com/questions/4309/can-you-have-a-css-class-name-with-a-space

@kof
Copy link
Member

kof commented Feb 9, 2019

Space is a valid css token that separates multiple class names, if you have a space in the class of the element it's considered as 2 classes

@bhupinderbola
Copy link
Author

@kof and @HenriBeck when developer is using displayName = "With Space", he/she is not expecting that react-jss will add a class like "With Space-root-0-1-2". We should either make it clear that developers can not use space in the displayName or when generating the className, we should replace space with underscore.

@kof
Copy link
Member

kof commented Feb 9, 2019

Yeah, we should just replace space with a dash.

@HenriBeck
Copy link
Member

Implemented in #1049

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It went crazy and killed everyone. complexity:low You can fix it, c'mon!
Projects
None yet
Development

No branches or pull requests

3 participants