javivelasco / react-css-themr

Easy theming and composition for CSS Modules.
MIT License
591 stars 68 forks source link

Uncaught TypeError: originalValue.split is not a function with RT-beta-6 #44

Closed landabaso closed 7 years ago

landabaso commented 7 years ago

I'm getting:

Uncaught TypeError: originalValue.split is not a function after updating RT from beta-4 to beta-6...

This error comes from function themeable. I debugged it a bit and it throws an error because it assumes that originalValue is a string.

However:typeof mixinValue === function, and typeof originalValue === function (when it throws).

EDIT: Now I see... These types are functions because I'm using isomorphic-style-loader.

The description of isomorphic-style-loader says it all:

An alternative CSS style loader, which works similarly to style-loader, but is optimized for isomorphic apps. In addition to what style-loader provides, it allow to render critical path CSS during server-side rendering (SSR), by adding two helper methods on to the styles object - ._insertCss() (injects CSS into the DOM) and ._getCss() (returns CSS string).

fredantell commented 7 years ago

I had the same issue due to using isomorphic-style-loader. Since I didn't fully understand the explanation above, for the sake of anyone else that stumbles on this: isomorphic-style-loader is adding two functions (insertCss() and getCss()) onto the CSS Module style object. Before this change react-css-themr was assuming the style object would always only contain keys that had strings as values. When it iterated over the object and found the functions injected by isomorphic-style-loader it threw an error.

I noticed that react-toolbox as of this writing is using 1.7.2 and not the 2.0.0 release which includes this fix for isomorphic-style-loader. I also see that react-toolbox is using some options that were deprecated in 2.0.0 as part of breaking changes.

Can we cherry pick the two related commits [e7e2126, 4e5f234] and release them as 1.7.3 and bump react-toolbox's dependency to that version?

landabaso commented 7 years ago

I'm using the beta6 of RT with themr 2.0. It's works fine.

landabaso commented 7 years ago

I read about the breaking changes (innerRef vs withRef). For some reason it did not affect me. May be you can try. There's a deprecation message that will be logged to the console if it conflicts with your code.

fredantell commented 7 years ago

I'm a little ignorant of how to get react-toolbox to install 2.0.0br6 when it lists 1.7.2 as a dependency. I believe I read that if I included react-css-themr 2.0.0 as an explicit dependency then if I blew away my node_modules and did a fresh npm install it may not install 1.7.2. I believe I can also edit my yarn.lock file or create an npm shrinkwrap. I feel a bit weird pushing this to my production though.

If you can describe the process for me I am happy to submit a PR for 1.7.3 with the relevant commits so that it works out of the box for the next person. Not sure how the tagging / release process works though.

I'd have submitted a PR to react-toolbox with a bump to 2.0.0 in the package.json until I saw that it was using the old withRef syntax.

Thanks for all your help already.