Beef up interpolation tests and interleave function to handle more edge cases

[emilgoldsmith]

I found a few edge cases, and while working on #59 I also just saw that tests for interleave were a bit lacking, so I felt like adding a few tests there that weren't braking but just for good form as well. I might have some more edge cases in mind as well. This is WIP but will resolve #62 and is related to #52.

It is branched off from #59 at the moment so until that is merged the diff here is a bit overkill.

[emilgoldsmith]

Added my proposed fix and put limitations in comments as this approach, though in my opinion much cleaner and better, still has some of the same drawbacks and assumptions the old one had, such as not supporting #13 (interpolating properties), interpolating several times to form one complete declaration etc. ${property}: ${value} would for example fail as well. But with a lot if not all of these I don't see any good way to do it without actually parsing the javascript and getting the values <.<. So I guess for now we should just keep assuming what is the most general practices (as I believe we currently are) and maybe then just add a bit more to the readme?

edit: I forgot to say, this is still WIP, just my fix is up if anyone wants to look at it and give feedback, still need to rewrite / write tests.

[emilgoldsmith]

Just remembered another one, it would also fail right now on something along the lines of

display: ${'block;'}
color: red;

aka when the semicolon is inside the interpolated value.

I still believe in what I said above, all these failing test cases I have, should I add them to some test file with "known problems" that we could somehow make not make travis stop us from pushing, or just add them to the README, or a third option?

[coveralls]

Coverage decreased (-0.008%) to 99.367% when pulling fa49dc9 on emilgoldsmith:fix/#62/one-line-css-interpolations into a4ce9db on styled-components:master.

[emilgoldsmith]

Okay I added some quite comprehensive tests. The test file is maybe nearing a size where it should be modularized though?

Also, I kept all the expressions' name properties even though they aren't technically needed with the changes I made in #59 regarding inserting dummyValue but I think it makes the tests more readable than just giving an array of empty object literals.

I just thought of some more edge cases just now regarding declaration blocks so I'll just add a few tests for that and modify the function I use to check if previous declaration was completed a tiny bit and then I think I'm pretty happy with this PR though it should not be merged in until #59 is as it's afollow up to that one.

[coveralls]

Coverage decreased (-0.004%) to 99.371% when pulling 400c41c on emilgoldsmith:fix/#62/one-line-css-interpolations into a4ce9db on styled-components:master.

[coveralls]

Coverage decreased (-0.004%) to 99.371% when pulling 870bc55 on emilgoldsmith:fix/#62/one-line-css-interpolations into a4ce9db on styled-components:master.

[emilgoldsmith]

I know this isn't actually necessary, but I was in the middle of adding it because I was erroneously recreating the css variable because I wanted to access all the previously processed text, but as I realized the css variable already did this I thought it was quite clean with the substitute variable and just left it there.

[emilgoldsmith]

Yeah so as I mentioned in the comments we don't actually need these anymore due to the dummyValues but I think it's pretty clear so I left them.

[emilgoldsmith]

IMO all is good now here except for waiting for #59 to be merged in before as this PR extends it.

edit: Oh yeah and also just btw, I think the only reason Coveralls is complaining is because there are now less lines in the general utils file so there the percentage of uncovered lines is less. I guess I could just add a test to that if (!match) statement if you want and coveralls would stop complaining.

[mxstbr]

Now there's conflicts, my bad 😢 Don't worry about coveralls we can make it less strict.

[emilgoldsmith]

Yep no problem, I'll resolve those with a quick rebase, that was expected when I branched off a different branch that I was also changing :).

[emilgoldsmith]

Yep a plain rebase didn't even have any conflicts, so everything should pass now.

The only thing I'm still curious about is whether we should add something to the README for this one? As in related to my previous comments:

not supporting #13 (interpolating properties), interpolating several times to form one complete declaration etc. ${property}: ${value} would for example fail as well. But with a lot if not all of these I don't see any good way to do it without actually parsing the javascript and getting the values <.<. So I guess for now we should just keep assuming what is the most general practices (as I believe we currently are) and maybe then just add a bit more to the readme?

Just remembered another one, it would also fail right now on something along the lines of

display: ${'block;'}
color: red;
aka when the semicolon is inside the interpolated value.

I still believe in what I said above, all these failing test cases I have, should I add them to some test file with "known problems" that we could somehow make not make travis stop us from pushing, or just add them to the README, or a third option?

I actually also had some more thoughts on how we could possibly handle these edge cases. But I have to get to work now haha ;). I'll open a discussion issue about a possible fix though.

[coveralls]

Coverage decreased (-0.008%) to 99.371% when pulling 1ea7f85 on emilgoldsmith:fix/#62/one-line-css-interpolations into 4706de2 on styled-components:master.

[mxstbr]

Thank you!!

I've updated coveralls to be less strict for the future 👌