Multiple versions of glamor

[otbe]

Hi @threepointone ,

we had a tricky bug in our codebase where several sub modules require a different version of glamor. In the specific case it was version 2.17.0 and 2.16.2. npm will install both versions (which is fine from a semver point of view), but this will lead to unexpected behavior. Both versions will be bundled by webpack and both will create a stylesheet instance.

If I use exported style({...})s from module B with glamor 2.17.0 in module A with glamor 2.16.2 it will lead to a runtime exception Cannot read property 'type' of undefined:

Is there anything we can do about this? Maybe introduce a better error message if something goes wrong?
I think the root cause is here, because it isLikeRule(x) (data-css-***) but its not in the cache. So this would be a good place to check for undefined? I think this would help to find subtile bugs.

If you want I can create a PR to add a check and error/warn message for dev/prod environment :)

Thanks for glamor 👍

[threepointone]

I'd be happy to take a PR that warns/errors about different versions of glamor / possible undefined. Please and thank you!

PS - I'm not sure otherwise how to 'solve' this issue. It's a problem with react et al iirc.

[otbe]

Will prepare one tomorrow :)
(its too late in germany right now :))

[threepointone]

btw, I'm always happy to hear more about how folks are using glamor. do you have any feedback for me? anything I can do better?

[otbe]

Currently we are shifting our codebase from less to glamor-only. As of now everything seems to work fine :)

Maybe a best practice guide would be nice to have. For example: we wonder how to compose styles the best/efficient/most performant way? -> Export plain objects vs style({..})s and so on...

Btw: I would like to add a hint in the README or somewhere to take care of only one instance of glamor. There was a caveats section a while ago, wasnt it? :)

[threepointone]

I'll take a PR for the caveat too!

[threepointone]

I'm hesitant to provide best practices just yet, but in general you should treat them like regular javascript 'values'.

[donaldpipowitch]

I'll create a different issue later to collect some example about best practice questions we currently have. Maybe there are some obvious things which could already be written into a best practice guide. If not we can at least identify the non-obvious parts :)

[threepointone]

live in 2.17.11, closing this one.

[thomhos]

Leaving this here for someone in the future :)

If you're using glamorous and using composition (e.g. glamorous(MyStyledDiv)( /* this needs to have styling rules */ )). You need to make sure it has styling rules. Just creating an instance and using it without styling seems to also trigger this error.

It's a weird scenario (creating an instance without styling), but I was prepping some components to have styling in the future when I ran into this issue.

Cheers,

[kitze]

@thomhos thanks for this, but how can you find this occurrence in the codebase?

[thomhos]

That might be hard to find depending on the situation. If it just glamorous(CustomThing)() you could spot them easily. But it could also be something that has styles based on props and if a combination of props causes the custom component to not receive any styles it can also cause this situation I believe.

So it really depends on how your using it, but for me the main thing I found out that it probably allocated an ID, doesn’t find styling and then assumes they’re missing.

Since working under this assumption and making sure there are always style rules if I make a new instance of a component, I haven’t run into the issue again.

[penx]

PR for a related issue that likely solves other Glamorous related issues that are throwing this error: #373

[getnorthern]

@thomhos You just saved me a world of pain! Thanks! 👍