Check for already-injected @font-face rules by src - Fixes #80

[jdan]

Hey @xymostech && @jlfwong,

This changeset modifies the call to injectStyleOnce to use src as the key, rather than fontName which does not need to be unique.

I added a quick test as well, let me know if you'd like it to be more thorough. You can also see the fix in action on my cloned Aphrodite output.

Input:

{
  "test": {
    fontFamily: [{
      fontStyle: "normal",
      fontWeight: "normal",
      fontFamily: "My Font",
      src: 'url(blah) format("woff"), url(blah) format("truetype")'
    }, {
      fontStyle: "italic",
      fontWeight: "normal",
      fontFamily: "My Font",
      src: 'url(blahitalic) format("woff"), url(blahitalic) format("truetype")'
    }]
  }
}

Current:

@font-face {
  font-style:normal;font-weight:normal;font-family:My Font;src:url(blah) format("woff"), url(blah) format("truetype");
}
.test_1pqrvdz {
  font-family:"My Font","My Font" !important;}

With fix (notice how "My Font" is uniquified in font-family):

@font-face {
  font-style:normal;font-weight:normal;font-family:My Font;src:url(blah) format("woff"), url(blah) format("truetype");
}
@font-face {
  font-style:italic;font-weight:normal;font-family:My Font;src:url(blahitalic) format("woff"), url(blahitalic) format("truetype");
}
.test_1pqrvdz {
  font-family:"My Font" !important;}

Maybe we want to think about only adding @font-face if it actually applies to the font-family rule? (i.e., only adding the second @font-family if we use font-style: italic?) dunno

[xymostech]

I'm not sure we guarantee that people have a Set polyfill, so I don't think this will work?

[jdan]

Good catch! Looks like our build script doesn't polyfill it.

[jlfwong]

I'm guessing this was already the plan, but I'd rather just not use Set than change this to include the polyfill (in the interest of keeping byte size down).

[jdan]

Yeah we're on the same page. Since outputting unique font-familys isn't important we can skip all of this.

[xymostech]

This fixes #80, correct?

[jdan]

Yep fixes #80! It's in the commit message so it'll auto-close if merged.

[jdan]

Kk after some revert-revert-reverts I removed the unique font style stuff. This is good for a re-review.

[jlfwong]

One line change accompanied by lots of tests -- this is my kinda change 😄

LGTM, but I only skimmed -- will leave to @xymostech for sign-off.

[smdern]

Any update on this?

[xymostech]

Looks like I totally dropped the ball here! Looks like this still merges, so I'm gonna squash+merge. Thanks for the reminder @smdern

[jlfwong]

Should this be a patch or a minor release?