thabti / react-native-css

Style React-Native components with css
MIT License
769 stars 66 forks source link

Fix handleRulesAndReturnCSSJSON returning undefined if any comments exist #17

Closed ExplodingCabbage closed 8 years ago

ExplodingCabbage commented 8 years ago

I'm trying to get this package working (it just outputs undefined as the stylesheet for me at the moment) and noticed this obvious bug. I'm not sure if what I've done here is sufficient to fix everything, but it can't hurt.

ExplodingCabbage commented 8 years ago

(Haven't yet tested this and not yet very familiar with ES 6, so if I'm being completely thick and the original code somehow worked then I apologise. It looks really obviously broken at a glance, though.)

thabti commented 8 years ago

@ExplodingCabbage can you post some css example of you getting undefined for? I personally never used continue. I will test it out and get back to you.

ExplodingCabbage commented 8 years ago

Without this fix it fails for literally any CSS that contains a comment within a selector block, like

.foo {
  background: pink;
  /* a comment */
}

The output from any such CSS is

module.exports = require('react-native').StyleSheet.create(undefined);
thabti commented 8 years ago

@ExplodingCabbage i'd guess it's the comment which is causing this issue.

I can't investigate right now, since I am at work.

ExplodingCabbage commented 8 years ago

By the way, I've now tested this and it works.

ExplodingCabbage commented 8 years ago

I guess you meant to merge this? Looks like you actually closed it without merging.

thabti commented 8 years ago

@ExplodingCabbage it seems to be working on my machine. It shouldn't continue when declaration.type isn't as expected.

ExplodingCabbage commented 8 years ago

declaration.type can be 'comment' if there are comments at the same level as rules, as in the example I gave. Currently the script responds to that by aborting and outputting 'undefined' as the stylesheet.

When you say it works on your machine, you mean that you can run react-native-css against

.foo {
  background: pink;
  /* a comment */
}

and get a proper RN stylesheet out? I don't know how that can be the case, unless a change in the behaviour of css-parse has caused us to have different behaviours on our different machines?

thabti commented 8 years ago

@ExplodingCabbage that is correct, I will merge this in, I did some testing and it doesn't really change the functionality.

https://circleci.com/gh/sabeurthabti/react-native-css/67

react-native-css@1.2.22

thabti commented 8 years ago

@ExplodingCabbage what i get back

module.exports = require('react-native').StyleSheet.create({"foo":{"background":"pink"}});