[v4] [icons] fix: icon paths are right-side up (#4984)

packages/core/src/components/icon/_icon.scss

@@ -31,9 +31,6 @@ $icon-classes: (
   > svg {
     // prevent extra vertical whitespace
     display: block;
-    // paths parsed by generate-icon-paths.js are mirrored vertically, so we need
-    // to flip them upright here
-    transform: scaleY(-1);
 
     // inherit text color unless explicit fill is set
     &:not([fill]) {

packages/core/src/components/icon/icon.tsx

@@ -150,12 +150,12 @@ export class Icon extends AbstractPureComponent2<IconProps & Omit<React.HTMLAttr
     }
 
     /** Render `<path>` elements for the given icon name. Returns `null` if name is unknown. */
-    private renderSvgPaths(pathsSize: number, iconName: IconName): JSX.Element | null {
+    private renderSvgPaths(pathsSize: number, iconName: IconName): JSX.Element[] | null {
         const svgPathsRecord = pathsSize === IconSize.STANDARD ? IconSvgPaths16 : IconSvgPaths20;
-        const pathString = svgPathsRecord[iconNameToPathsRecordKey(iconName)];
-        if (pathString == null) {
+        const paths = svgPathsRecord[iconNameToPathsRecordKey(iconName)];
+        if (paths == null) {
             return null;
         }
-        return <path d={pathString} fillRule="evenodd" />;
+        return paths.map((path, i) => <path key={i} d={path} fillRule="evenodd" />);
     }
 }

packages/docs-app/src/examples/core-examples/common/iconNames.ts

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2021 Palantir Technologies, Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { IconName, IconNames } from "@blueprintjs/icons";
+
+export const NONE = "(none)";
+export type IconNameOrNone = IconName | typeof NONE;
+
+export function getIconNames(): IconNameOrNone[] {
+    const iconNames = new Set<IconNameOrNone>();
+    for (const [, name] of Object.entries(IconNames)) {
+        iconNames.add(name);
+    }
+    iconNames.add(NONE);
+    return Array.from(iconNames.values());
+}

packages/docs-app/src/examples/core-examples/common/iconSelect.tsx

@@ -17,20 +17,19 @@
 import * as React from "react";
 
 import { Alignment, Button, Classes, MenuItem } from "@blueprintjs/core";
-import { IconName, IconNames } from "@blueprintjs/icons";
+import { IconName } from "@blueprintjs/icons";
 import { ItemRenderer, Select } from "@blueprintjs/select";
 
+import { getIconNames, IconNameOrNone, NONE } from "./iconNames";
+
+const ICON_NAMES = getIconNames();
+
 export interface IIconSelectProps {
     iconName?: IconName;
     onChange: (iconName?: IconName) => void;
 }
 
-const NONE = "(none)";
-type IconType = IconName | typeof NONE;
-const ICON_NAMES = Object.keys(IconNames).map<IconType>((name: string) => IconNames[name as keyof typeof IconNames]);
-ICON_NAMES.push(NONE);
-
-const TypedSelect = Select.ofType<IconType>();
+const TypedSelect = Select.ofType<IconNameOrNone>();
 
 export class IconSelect extends React.PureComponent<IIconSelectProps> {
     public render() {
@@ -84,5 +83,5 @@ export class IconSelect extends React.PureComponent<IIconSelectProps> {
         return iconName.toLowerCase().indexOf(query.toLowerCase()) >= 0;
     };
 
-    private handleIconChange = (icon: IconType) => this.props.onChange(icon === NONE ? undefined : icon);
+    private handleIconChange = (icon: IconNameOrNone) => this.props.onChange(icon === NONE ? undefined : icon);
 }

packages/icons/package.json

@@ -50,7 +50,7 @@
         "react": "^16.14.0",
         "react-dom": "^16.14.0",
         "react-test-renderer": "^16.14.0",
-        "svg-parser": "^2.0.4",
+        "svgo": "^1.3.2",
         "typescript": "~4.1.2",
         "webpack-cli": "^3.3.12"
     },

packages/icons/scripts/common.js

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2021 Palantir Technologies, Inc. All rights reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+const fs = require("fs");
+const path = require("path");
+
+const COPYRIGHT_HEADER = "/*\n * Copyright 2021 Palantir Technologies, Inc. All rights reserved.\n */\n";
+const RESOURCES_DIR = path.resolve(__dirname, "../../../resources/icons");
+const GENERATED_SRC_DIR = path.resolve(__dirname, "../src/generated");
+const NS = "bp4";
+
+/**
+ * Writes lines to given filename in GENERATED_SRC_DIR.
+ *
+ * @param {string} filename
+ * @param {Array<string>} lines
+ */
+function writeLinesToFile(filename, ...lines) {
+    const outputPath = path.join(GENERATED_SRC_DIR, filename);
+    const contents = [COPYRIGHT_HEADER, ...lines, ""].join("\n");
+    fs.writeFileSync(outputPath, contents);
+}
+
+module.exports = {
+    COPYRIGHT_HEADER,
+    RESOURCES_DIR,
+    GENERATED_SRC_DIR,
+    NS,
+    writeLinesToFile,
+};

packages/icons/scripts/generate-icon-fonts.js

@@ -22,11 +22,9 @@ const { getLogger } = require("fantasticon/lib/cli/logger");
 const fs = require("fs");
 const path = require("path");
 
-const RESOURCES_DIR = path.resolve(__dirname, "../../../resources/icons");
-const GENERATED_SRC_DIR = path.resolve(__dirname, "../src/generated");
-const logger = getLogger();
-const NS = "bp4";
+const { RESOURCES_DIR, GENERATED_SRC_DIR, NS } = require("./common");
 
+const logger = getLogger();
 logger.start();
 
 fs.mkdirSync(path.join(GENERATED_SRC_DIR, `16px/paths`), { recursive: true });

packages/icons/scripts/generate-icon-paths.js

@@ -21,67 +21,68 @@
 const { camelCase } = require("change-case");
 const fs = require("fs");
 const path = require("path");
-const { parse } = require("svg-parser");
-
-const GENERATED_SRC_DIR = path.resolve(__dirname, "../src/generated");
-const COPYRIGHT_HEADER = "/*\n * Copyright 2021 Palantir Technologies, Inc. All rights reserved.\n */\n";
-
-for (const iconSize of [16, 20]) {
-    const iconFontSvgDocument = fs.readFileSync(
-        path.join(GENERATED_SRC_DIR, `${iconSize}px/blueprint-icons-${iconSize}.svg`),
-        "utf8",
-    );
-
-    const icons = [];
-    console.info(`Parsing SVG glyphs from generated ${iconSize}px SVG icon font...`);
-    parseIconGlyphs(iconFontSvgDocument, (iconName, iconPath) => {
-        icons.push(iconName);
-        writeLinesToFile(`${iconSize}px/paths/${iconName}.ts`, `const path = "${iconPath}"`, "export default path;");
-    });
-    console.info(`Parsed ${icons.length} icons.`);
-
-    console.info(`Writing index file for ${iconSize}px icon kit paths...`);
-    writeLinesToFile(
-        `${iconSize}px/paths/index.ts`,
-        ...icons.map(iconName => `export { default as ${camelCase(iconName)} } from "./${iconName}";`),
-    );
-    console.info("Done.");
-}
+// Note: we had issues with this approach using svgo v2.x, so for now we stick with v1.x
+// With v2.x, some shapes within the icon SVGs would not get converted to paths correctly,
+// resulting in invalid d="..." attributes rendered by the <Icon> component.
+const SVGO = require("svgo");
 
 /**
- * Parse all icons of a given size from the SVG font generated by fantasticon.
- * At this point we've already optimized the icon SVGs through svgo (via fantasticon), so
- * we avoid duplicating that work by reading the generated glyphs here.
- *
- * @param {string} iconFontSvgDocument
- * @param {(iconName: string, iconPath: string) => void} cb iterator for each icon path
+ * @typedef {Object} IconMetadata
+ * @property {string} displayName - "Icon name" for display
+ * @property {string} iconName - `icon-name` for IconName and CSS class
+ * @property {string} tags - comma separated list of tags describing this icon
+ * @property {string} group - group to which this icon belongs
+ * @property {string} content - unicode character for icon glyph in font
  */
-function parseIconGlyphs(iconFontSvgDocument, cb) {
-    const rootNode = parse(iconFontSvgDocument);
-    const defs = rootNode.children[0].children[0];
-    const glyphs = defs.children[0].children.filter(node => node.tagName === "glyph");
 
-    for (const glyph of glyphs) {
-        const name = glyph.properties["glyph-name"];
+/** @type {IconMetadata[]} */
+const ICONS_METADATA = require("../icons.json").sort((a, b) => a.iconName.localeCompare(b.iconName));
+const { RESOURCES_DIR, writeLinesToFile } = require("./common");
+
+const svgo = new SVGO({ plugins: [{ convertShapeToPath: { convertArcs: true } }] });
+const ICON_NAMES = ICONS_METADATA.map(icon => icon.iconName);
+
+(async () => {
+    for (const iconSize of [16, 20]) {
+        const iconPaths = await getIconPaths(iconSize);
 
-        // HACKHACK: for some reason, there are duplicates with the suffix "-1", so we ignore those
-        if (name.endsWith("-1")) {
-            continue;
+        for (const [iconName, pathStrings] of Object.entries(iconPaths)) {
+            writeLinesToFile(
+                `${iconSize}px/paths/${iconName}.ts`,
+                `const paths: string[] = [${pathStrings.join(", ")}];`,
+                "export default paths;",
+            );
         }
 
-        const path = glyph.properties["d"];
-        cb(name, path);
+        console.info(`Writing index file for ${iconSize}px icon kit paths...`);
+        writeLinesToFile(
+            `${iconSize}px/paths/index.ts`,
+            ...ICON_NAMES.map(iconName => `export { default as ${camelCase(iconName)} } from "./${iconName}";`),
+        );
+        console.info("Done.");
     }
-}
+})();
 
 /**
- * Writes lines to given filename in GENERATED_SRC_DIR.
+ * Loads SVG file for each icon, extracts path strings `d="path-string"`,
+ * and constructs map of icon name to array of path strings.
  *
- * @param {string} filename
- * @param {Array<string>} lines
+ * @param {16 | 20} iconSize
  */
-function writeLinesToFile(filename, ...lines) {
-    const outputPath = path.join(GENERATED_SRC_DIR, filename);
-    const contents = [COPYRIGHT_HEADER, ...lines, ""].join("\n");
-    fs.writeFileSync(outputPath, contents);
+async function getIconPaths(iconSize) {
+    /** @type Record<string, string[]> */
+    const iconPaths = {};
+    for (const iconName of ICON_NAMES) {
+        const filepath = path.join(RESOURCES_DIR, `${iconSize}px/${iconName}.svg`);
+        const svg = fs.readFileSync(filepath, "utf-8");
+        const optimizedSvg = await svgo.optimize(svg, { path: filepath });
+        const pathStrings = (optimizedSvg.data.match(/ d="[^"]+"/g) || [])
+            // strip off leading 'd="'
+            .map(s => s.slice(3))
+            // strip out newlines and tabs, but keep other whitespace
+            .map(s => s.replace(/[\n\t]/g, ""));
+        iconPaths[iconName] = pathStrings;
+    }
+    console.info(`Parsed ${Object.keys(iconPaths).length} ${iconSize}px icons.`);
+    return iconPaths;
 }

packages/node-build-scripts/package.json

@@ -28,7 +28,6 @@
         "strip-css-comments": "^4.1.0",
         "stylelint": "~13.8.0",
         "stylelint-junit-formatter": "^0.2.2",
-        "svgo": "^1.3.2",
         "tslint": "~6.1.3",
         "typescript": "~4.1.2",
         "yargs": "^17.1.1"

yarn.lock

@@ -12492,11 +12492,6 @@ supports-color@^6.1.0:
   dependencies:
     has-flag "^3.0.0"
 
-svg-parser@^2.0.4:
-  version "2.0.4"
-  resolved "https://registry.yarnpkg.com/svg-parser/-/svg-parser-2.0.4.tgz#fdc2e29e13951736140b76cb122c8ee6630eb6b5"
-  integrity sha512-e4hG1hRwoOdRb37cIMSgzNsxyzKfayW6VOflrwvR+/bzrkyxY/31WkbgnQpgtrNp1SdpJvpUAGTa/ZoiPNDuRQ==
-
 svg-pathdata@^5.0.0:
   version "5.0.5"
   resolved "https://registry.yarnpkg.com/svg-pathdata/-/svg-pathdata-5.0.5.tgz#65e8d765642ba15fe15434444087d082bc526b29"