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

[enhancement]. using trim() on colors definitions and references to prevent incorrect spaces from causing unresolved #1435

Closed
ash0080 opened this issue Jan 11, 2021 · 6 comments · Fixed by #2099
Labels
enhancement New feature or request released This issue/pull request has been released.

Comments

@ash0080
Copy link

ash0080 commented Jan 11, 2021

Otherwise it is very difficult to find such bugs

@ash0080
Copy link
Author

ash0080 commented Jan 11, 2021

Since the styled component syntax has to support both color aliases and native arguments, I don't think it's realistic to check for errors, so at least reduce the number of such hard-to-find errors

@ash0080 ash0080 changed the title [enhancement]. using trim on colors definitions and references to prevent incorrect spaces from causing unresolved [enhancement]. using trim() on colors definitions and references to prevent incorrect spaces from causing unresolved Jan 11, 2021
@lachlanjc
Copy link
Member

To clarify, you want e.g. color: ' red' to work by trimming the extra space? It makes sense, but could you explain the reason for this?

@hasparus
Copy link
Member

hasparus commented Jan 12, 2021

Alternatively, a solution I think I'd prefer, we could throw on such an error.

if (process.env.NODE_ENV !== 'production') {
  if (color in theme.colors && color !== color.trim()) {
    throw new Error("serious message")
  }
}

@ash0080
Copy link
Author

ash0080 commented Jan 14, 2021

Alternatively, a solution I think I'd prefer, we could throw on such an error.

if (process.env.NODE_ENV !== 'production') {
  if (color in theme.colors && color !== color.trim()) {
    throw new Error("serious message")
  }
}

Mostly it can't be a typo in the config

@ash0080
Copy link
Author

ash0080 commented Jan 14, 2021

To clarify, you want e.g. color: ' red' to work by trimming the extra space? It makes sense, but could you explain the reason for this?

'red' is a native variable, Perhaps not an appropriate example

Not a native variable, but reference, Extra spaces are easier to find in the front, harder in the back
much more harder when using in array format

Sometimes formatting plugins such as prettier to insert incorrect spaces when the code you are currently writing is not completely closed, so it is not uncommon

I think adding a trim() is one of the easiest ways to implement it, it doesn't avoid errors 100% of the time, but at least 95% of the time it's possible

Imagine when your component has more than 200 lines of code or more, with 3-4 sx blocs, it is hard to find a small space that causes an element not to render correctly
Then consider why stylesheets almost ignore spaces.

eg:

bg: ['body ', 'primary' ] 

@flo-sch flo-sch added the enhancement New feature or request label Jun 14, 2021
@hasparus hasparus added the prerelease This change is available in a prerelease. label Jan 29, 2022
@hasparus
Copy link
Member

🚀 Issue was released in v0.14.0 🚀

@hasparus hasparus added released This issue/pull request has been released. and removed prerelease This change is available in a prerelease. labels Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants