-
Notifications
You must be signed in to change notification settings - Fork 199
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
Failing test and proposed fix for css({'data-css-nil': ''}) #373
base: master
Are you sure you want to change the base?
Conversation
Thanks for opening this! This is a great first step to figuring out what's up! I personally don't have time to help with this, and I don't understand the inner-workings of glamor super-well, but hopefully someone can help. |
I was starting to understand it earlier. Will take another look tomorrow |
Seems like it was originally added to fix #115 |
I've added a proposed fix to this PR that makes |
Another scenario which would be handy to fix is |
Travis failure is unrelated:
|
Hey @penx, since we at Signavio believe in this library we want to continue developing it further. Therefore, we've forked it and will release it as |
I can't see any documentation or explanation in the code as to what
data-css-nil
is for, other than:glamor/src/index.js
Lines 430 to 432 in db533d7
...and some perhaps related discussion in issue #303.
However, Glamorous, under certain conditions, will call
css({'data-css-nil': ''})
, which causes Glamor to throw the following error:See:
paypal/glamorous#396
paypal/glamorous#397
I am not sure if this bug is due to Glamorous sending something through that it shouldn't, or Glamor not handling this correctly, so have created failing tests for both scenarios.
I can't find references to
data-css-nil
in Glamorous' code base but can continue to debug to try find out what's causing this. However if anyone has any insight on this that may save a lot of code reading then please let me know 🙂