aggressive color checking

[MrLoh]

Hi, first of all. Thank you for this library.

I've build a little styled-components replacement for react-native for our company, that adds some features like direct access to theme variables by name.

The heavy lifting is provided by this library. Now in 3a3a596 you made the check for color variables on borderColor much more aggressive. Before it would simply give me an object with whatever I had entered (like a color name from my scheme) and I could then resolve that name to the actual color with my library. Now I get an error.

Is there a chance to allow normal words again as color values, or would you accept a PR that would allow adding valid color values via a configuration function.

[jacobp100]

I generally want to avoid adding workarounds in this parser. Usually, these sort of things can be done by doing some pre-parsing before you run css-to-react-native

Since you know your theme parameter names, you can just do a regexp find and replace on the CSS before it is passed in (or better yet, do it on only the values of the tuples you pass in)

Let me know if that helps!

[MrLoh]

@jacobp100 the problem with doing preprocessing is performance. It's much faster to run over the already parsed data structures compared to processing the strings twice, so I'm really trying to avoid this.

[jacobp100]

I think you won’t see any performance drop doing pre-processing vs post-processing

Looking at your project, the biggest cost is probably this library

If you’re concerned about performance, you could try cssta. It does ahead of time compilation for everything so almost nothing happens at runtime

[MrLoh]

Cssta looks very impressive. But it doesn’t seem to be a superset of styled components (no function interpolation for props access, which is probably what allows ahead of time compilation). And we cannot rewrite our whole codebase.

Especially for color values the complexity is quite high. Now I can check if a key describes a color prop an wether it’s value Is in the theme. Then I’d have to check wether any of the color values from my theme appear in the string, because shorthand’s are not resolved yet. Or I rebuilt the functionality of this library, to extract the color value and then do what I was doing before.

It would be very easy on the other hand to make a PR here to allow providing custom token types with a global setTokenType function or something. I understand that it adds surface area to the lib, which is always undesired. But this lib is for tool authors mostly, so configurability is important. All I want, is to add less or sass style variables. Which seems like nothing super extraordinary.

[MrLoh]

I guess I could prefix color variables with an identifier like $ which would make them relatively easy to parse.

[jacobp100]

It’s more for correctness I don’t want to change the parser. What if you wrote border: 1px solid dashed, one of those should—by this proposal—be a colour. But which?

Anyway, I think you have a route forward! I’ll close this issue, because I don’t think there’s anything we can do in this library, but feel free to continue in this thread if you have any more questions 🙂

[MrLoh]

@jacobp100 I tried to implement it and actually there is no way for me to get around this. The whole performance of my library is based on the fact that the hard work is performed by this library, but that I can cache the result of the css parsing based on the cssString. I can do that on load of my code (and I could precompile it in the future) for styles that are independent of props input and I can memoize it on mount for components that do need to use variables. But I just need to do it once.

Now if the cssString is not a constant anymore, but a function of theme => cssString, then I need to do all the hard work on mount for every component and could never precompile. This is because my variables are not static, but can be changed (for example to enable dark mode). So there is no way to preprocess the component and keep performance.

I understand that you want to ensure correctness, and thus don't want to water down the definition of color. But then the best way would be to allow users of the library to overwrite the color tokenType. I'd be happy to provide a PR. But otherwise I don't see a solution for my issue.

[MrLoh]

If I can overwrite the color matcher, than I could ensure correct parsing, by only additionally allowing color values that contain a prefix like $.

[jacobp100]

Am I right in thinking you parse the entire string once and then all subsequent updates are done on the output?

I don’t really want to allow editing tokens etc., because that will make refactoring harder or introduce breaking changes later down the line. If you’re set on this parser taking custom tokens, I think you will have to run a fork

Alternatively you could generate a random hex code for each theme colour (not already used in the string), parse it, and then make a mapping of properties to theme colours. You might even get a perf win because by doing that you have the smallest mutation set to change the style object. Just an idea anyway!

[MrLoh]

Yes I parse the entire string once.

Maybe your idea could work. It seems you allow 8 digit react native style rgba hex values and color functions. So maybe I can hijack the transparent 8 digit hex values.

css-to-react-native/src/tokenTypes.js

Line 13 in 26f4b49

const hexColorRe = /^(#(?:[0-9a-f]{3,4}){1,2})$/i

[MrLoh]

You’re not gonna start preventing rem and vh/vw unit values in the future though right. Because that would completely kill it for me.

The idea is that this library follows the CSS spec right. And not just the subset of enabled react native pseudo CSS.

[jacobp100]

I think realistically those extra units are here to stay. Quite a few projects use them. They don’t seem to be causing problems when parsing, and have been around for a while. If we found cases where they caused issues in parsing, we might pull the plug on them, but that would at least be marked as a breaking change

The aim is actually just the subset of CSS react-native supports. We actually extend CSS in some parts for RN-only properties

[MrLoh]

Well this just blew up in my face, as there is no easy way to guarantee that the variables are set for pre processing before the styles are resolved as that depends on execution order of code, which I cannot control in a framework like next js. Are you aware of any alternatives to this library? Otherwise I will go with forking.

[jacobp100]

I'm afraid this is the only one. You might be able to get away with some webpack trickery with webpack-plugin-replace and this line

Hope that helps!

[MrLoh]

Thanks. How do you get around the problem that variable resolution depends on execution order of the code, in CSSTA. Or does CSSTA do the preprocessing on render?

[jacobp100]

Cssta does 99% of processing at compile time - and in a lot of cases, this library won't even be imported in the final code.

But there are edge cases. In most edge cases, we resolve this once at code execution time. In some of the worse edge cases, it is wrapped in a React.useMemo - but these almost never show up