[Fix] #196

[bogas04]

As per @mitchellhamilton's suggestion, removing this part should be good enough.

What: As per @mitchellhamilton #196 (comment), the if and else block is now just changed the contents of else block.

Why: This was causing an error on importing injectGlobal from 'emotion'.

How: As per @mitchellhamilton "The optimisation ... isn't needed since uglify will remove the undefined expression anyway."

Checklist:

• Documentation
• Tests
• Code complete

[tkh44]

@bogas04 can you please fill out basic information of this PR. It has a very vague title and none of the fields are filled out. I am much more likely to merge things that I can get the entire context of the situation.

Thanks!

[thangngoc89]

@tkh44 I think it's per @mitchellhamilton request here #196 (comment)

[tkh44]

I understand that @thangngoc89, but I'm just asking nicely to use the provided template so that @mitchellhamilton and I can quickly scan the PR to get some context.

[bogas04]

@tkh44 Sorry for being lazy! I've added more details to the description. I often use commit messages like [Fix] #issuenumber as they close the issue and also carry information in the issuenumber. But you're right, I should've filled the basic information.

[bogas04]

@mitchellhamilton I think the fix isn't working as intended. We output undefined; with this. I'll try to investigate further.

[emmatown]

@bogas04 it should output undefined. As I said the optimisation I was trying to do by removing the undefined isn't needed.

[bogas04]

@mitchellhamilton Okay I understand. In that case, are we supposed to update the tests ?

[emmatown]

Yes

[bogas04]

@mitchellhamilton

Something like this in font-face.test.js.snap#L3

// Jest Snapshot v1, https://goo.gl/fbAQLP

- exports[`fontFace babel extract basic 1`] = `"import \\"./font-face.test.emotion.css\\";"`;
+ exports[`fontFace babel extract basic 1`] = `
+ "import \\"./font-face.test.emotion.css\\";"
+
+ undefined;
+ `; 

And in inject-global.test.js.snap#L22 as well.

- exports[`babel injectGlobal extract injectGlobal basic 1`] = `"import \\"./inject-global.test.emotion.css\\";"`;
+ exports[`babel injectGlobal extract injectGlobal basic 1`] = `
+ "import \\"./inject-global.test.emotion.css\\";"
+
+ undefined;
+ `;

?

[emmatown]

Yes, that's exactly what it should be.

[bogas04]

perfect, onto it 👍

[emmatown]

It looks like you updated the snapshots manually and they aren't don't match. I would recommend reading this to understand how snapshot tests work but basically press u when you're in watch mode and the snapshots will update.

[codecov-io]

Codecov Report

Merging #214 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #214      +/-   ##
=========================================
- Coverage   90.12%   90.1%   -0.03%     
=========================================
  Files          22      22              
  Lines         952     950       -2     
  Branches      255     254       -1     
=========================================
- Hits          858     856       -2     
  Misses         76      76              
  Partials       18      18

Impacted Files	Coverage Δ
src/babel.js	97.76% <100%> (-0.04%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0c1427...869f6a9. Read the comment docs.

[bogas04]

Thank you for supporting @mitchellhamilton 🎉