omacranger / fontawesome-subset

Creates subsets of FontAwesome fonts for optimized use on the web.
GNU General Public License v3.0
66 stars 15 forks source link

FA v5.10.2 Duotone some icons fail to subset #15

Closed optimuspwnius closed 2 years ago

optimuspwnius commented 2 years ago

Some duotone icons fail subsetting. For example:

  fontawesomeSubset({
    duotone: ['sort']  
  },
  'sass/webfonts',
  {
      package: 'pro'
  });

Results in:

/node_modules/svg2ttf/lib/svg.js:18
    if (pathElem.hasAttribute('d')) {
                 ^
TypeError: Cannot read properties of undefined (reading 'hasAttribute')
    at getGlyph (/node_modules/svg2ttf/lib/svg.js:18:18)
    at /node_modules/svg2ttf/lib/svg.js:160:17
    at /node_modules/lodash/lodash.js:4943:15
    at Function.forEach (/node_modules/lodash/lodash.js:9410:14)
    at Object.load (/node_modules/svg2ttf/lib/svg.js:159:5)
    at svg2ttf (/node_modules/svg2ttf/index.js:22:21)
    at fontawesomeSubset (node_modules/fontawesome-subset/dist/index.js:101:48)

Looking at Line 99 in index.js

const svgContentsNew = svgFile.replace(new RegExp(`(<glyph glyph-name="(${glyphsToRemove.join("|")})".*?\\/>)`, "gms"), "").replace(/>\s+</gms, "><");

svgContentsNew for this svg outputs 2 glyphs, "sort-primary" and "sort-secondary". I believe svg2ttf fails on some duotone icons because one of the glyphs have no "d" attribute. Here's a sample of the output:

<?xml version="1.0" standalone="no"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" ><svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1"><metadata>
Created by FontForge 20201107 at Wed Aug  4 12:24:16 2021
 By Robert Madole
Copyright (c) Font Awesome
</metadata><!-- Font Awesome Pro 5.15.4 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license (Commercial License) --><defs><font id="FontAwesome5Duotone-Solid"><font-face 
    font-family="Font Awesome 5 Duotone Solid"/><missing-glyph /><glyph glyph-name="sort-primary" unicode="&#xf0dc;"/><glyph glyph-name="sort-secondary" unicode="&#x10f0dc;" d="..." /></font></defs></svg>

Edit: Removed some icon specific stuff, I shouldn't have pasted the full icon output here due to license restrictions.

omacranger commented 2 years ago

@optimuspwnius Hey there! Thanks for the report. Sorry it's taken so long to get back to you. This fell off my radar a bit.

You're absolutely right. Because the SVG element doesn't have a d attribute, it's falling back to attempting to find a nested path element and throwing an error when attempting to access hasAttribute on that, which is undefined. I'll take a look here and see if excluding items with no d attribute would work, or if FontAwesome's CSS is relying on that somehow for rendering.

omacranger commented 2 years ago

It does look like they are still referencing them, despite the paths being blank. I'll see what I can do about inserting an empty d="" attribute on glyphs that do not have it, so we can still generate those fonts as expected.