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

Different output in node vs in browser? #234

Closed
ghost opened this issue Nov 2, 2022 · 12 comments
Closed

Different output in node vs in browser? #234

ghost opened this issue Nov 2, 2022 · 12 comments

Comments

@ghost
Copy link

ghost commented Nov 2, 2022

This may be more of a question than a bug report, but I can't find this in the docs. I'm precompiling my color manipulation so it doesn't bog runtime and I'm getting a different toString() output in node vs in browser.

new Color('#ffff33').to('hsl').toString()

Outputs strings color(hsl 60 100 60) in node and hsl(60 100% 60%) in browser. What's the intended way to get the CSS-accepted format in node?

@LeaVerou
Copy link
Member

LeaVerou commented Nov 2, 2022

Looks like for some reason spaces/hsl.js is not imported in Node so it falls back to the default output. Could you provide more details about how you are importing Color.js? Are you importing files manually or using a bundle?

@ghost
Copy link
Author

ghost commented Nov 2, 2022

Thanks for the insane reply speed. I'm using it inside Vanilla Extract *.css.ts files (theme specifically) kind of like so (not actual implementation)

// theme.css.ts
import Color from 'colorjs.io'

export const styles = style({
   color: new Color('#ffff33').to('hsl').toString()
})

@LeaVerou
Copy link
Member

LeaVerou commented Nov 2, 2022

Wait, if HSL was not included at all, it wouldn't know how to do the conversion either. The plot thickens… Are you using the same version in both cases?

@ghost
Copy link
Author

ghost commented Nov 2, 2022

Yes, 0.4.1

@ghost
Copy link
Author

ghost commented Nov 3, 2022

Could I help here any further? Maybe a reproduction vite stack?

@LeaVerou
Copy link
Member

LeaVerou commented Nov 3, 2022

The less tooling that is needed to reproduce it, the better. E.g. can you reproduce it in plain Node? I have never used vanilla-extract or vite, so I'm not sure where to even start debugging this 😕

Oh, what happens if you call .toString({format: "hsl"})?

@ghost
Copy link
Author

ghost commented Nov 3, 2022

In 0.4.1 toString expects 0 arguments, but in 0.4.0 setting the format option did basically nothing, the resulting string was the same as without it.

@ghost
Copy link
Author

ghost commented Nov 3, 2022

Hmm, actually I can't reproduce it in pure node, the output is correct as hsl(60 100% 60%). Could be somehow related to Vanilla Extract which does some pre-compiling on the .css.ts files. I'll create some minimal environment where the problem is reproducible.

@ghost
Copy link
Author

ghost commented Nov 3, 2022

https://github.com/mystrdat/colorjs-vanilla-test

Barebones vite react-ts template just with vanilla-extract added. There are 2 logs, one in https://github.com/mystrdat/colorjs-vanilla-test/blob/master/src/App.css.ts which outputs color(hsl 60 100 60) in the terminal process (doesn't run in browser) a then one in https://github.com/mystrdat/colorjs-vanilla-test/blob/master/src/App.tsx which runs in browser and outputs hsl(60 100% 60%) correctly.

I'll see if I can reduce it further but given it works in node it does seem somehow related to https://vanilla-extract.style

@LeaVerou
Copy link
Member

LeaVerou commented Nov 3, 2022

Thank you so much for tracking this down!

@ghost
Copy link
Author

ghost commented Nov 3, 2022

@LeaVerou This seems to have some insight vanilla-extract-css/vanilla-extract#899 (comment)

@ghost
Copy link
Author

ghost commented Nov 4, 2022

So to summarize, it seems that the gist of the issue is this bug that's overwriting your processFormat function with the getPath function in esbuild, which was fixed in 0.12.17. Vite uses a recent version but Vanilla Extract is still on 0.11.*. Hopefully the authors will update soon. Case closed, thank you very much for your time Lea!

@ghost ghost closed this as completed Nov 4, 2022
This issue was closed.
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

No branches or pull requests

1 participant