Fix outline icon caching

Summary:
The build-release script was using different logic to construct the local url, than the lookup file.
Changed so they both use the same function.
And unit tests added.

Reviewed By: passy

Differential Revision: D17602476

fbshipit-source-id: 6aaedd58eafb2cc59adcdc0ebb4dd329bf99c33a

scripts/build-release.js

@@ -18,7 +18,11 @@ const {
   genMercurialRevision,
 } = require('./build-utils.js');
 const fetch = require('node-fetch');
-const {ICONS, getIconURL} = require('../src/utils/icons.js');
+const {
+  ICONS,
+  buildLocalIconPath,
+  getIconURL,
+} = require('../src/utils/icons.js');
 
 function generateManifest(versionNumber) {
   const filePath = path.join(__dirname, '..', 'dist');
@@ -137,11 +141,7 @@ function downloadIcons(buildFolder) {
           res =>
             new Promise((resolve, reject) => {
               const fileStream = fs.createWriteStream(
-                path.join(
+                path.join(buildFolder, buildLocalIconPath(name, size, density)),
-                  buildFolder,
-                  'icons',
-                  `${name}-${size}@${density}x.png`,
-                ),
               );
               res.body.pipe(fileStream);
               res.body.on('error', reject);

src/utils/__tests__/icons.node.js

@@ -0,0 +1,32 @@
+/**
+ * Copyright 2018-present Facebook.
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ * @format
+ */
+
+import {buildLocalIconPath, buildIconURL} from '../icons';
+
+test('filled icons get correct local path', () => {
+  const path = buildLocalIconPath('star', 12, 2);
+  expect(path).toBe('icons/star-filled-12@2x.png');
+});
+
+test('outline icons get correct local path', () => {
+  const path = buildLocalIconPath('star-outline', 12, 2);
+  expect(path).toBe('icons/star-outline-12@2x.png');
+});
+
+test('filled icons get correct URL', () => {
+  const path = buildIconURL('star', 12, 2);
+  expect(path).toBe(
+    'https://external.xx.fbcdn.net/assets/?name=star&variant=filled&size=12&set=facebook_icons&density=2x',
+  );
+});
+
+test('outline icons get correct URL', () => {
+  const path = buildIconURL('star-outline', 12, 2);
+  expect(path).toBe(
+    'https://external.xx.fbcdn.net/assets/?name=star&variant=outline&size=12&set=facebook_icons&density=2x',
+  );
+});

src/utils/icons.js

@@ -14,6 +14,35 @@ const fs = require('fs');
 const path = require('path');
 const {remote} = require('electron');
 
+// Takes a string like 'star', or 'star-outline', and converts it to
+// {trimmedName: 'star', variant: 'filled'} or {trimmedName: 'star', variant: 'outline'}
+function getIconPartsFromName(icon) {
+  const isOutlineVersion = icon.endsWith('-outline');
+  const trimmedName = isOutlineVersion ? icon.replace('-outline', '') : icon;
+  const variant = isOutlineVersion ? 'outline' : 'filled';
+  return {trimmedName: trimmedName, variant: variant};
+}
+
+// $FlowFixMe not using flow in this file
+function buildLocalIconPath(name, size, density) {
+  const icon = getIconPartsFromName(name);
+
+  return path.join(
+    'icons',
+    `${icon.trimmedName}-${icon.variant}-${size}@${density}x.png`,
+  );
+}
+
+// $FlowFixMe not using flow in this file
+function buildIconURL(name, size, density) {
+  const icon = getIconPartsFromName(name);
+  return `https://external.xx.fbcdn.net/assets/?name=${
+    icon.trimmedName
+  }&variant=${
+    icon.variant
+  }&size=${size}&set=facebook_icons&density=${density}x`;
+}
+
 module.exports = {
   ICONS: {
     'arrow-right': [12],
@@ -38,6 +67,9 @@ module.exports = {
     'star-outline': [16, 24],
   },
 
+  buildLocalIconPath: buildLocalIconPath,
+  buildIconURL: buildIconURL,
+
   // $FlowFixMe: not using flow in this file
   getIconURL(name, size, density) {
     if (name.indexOf('/') > -1) {
@@ -73,16 +105,7 @@ module.exports = {
       }
     }
 
-    let variant = 'filled';
+    const localPath = buildLocalIconPath(name, size, density);
-    if (name.endsWith('-outline')) {
-      name = name.replace('-outline', '');
-      variant = 'outline';
-    }
-
-    const localPath = path.join(
-      'icons',
-      `${name}-${variant}-${size}@${density}x.png`,
-    );
     // resolve icon locally if possible
     if (
       remote &&
@@ -90,6 +113,6 @@ module.exports = {
     ) {
       return localPath;
     }
-    return `https://external.xx.fbcdn.net/assets/?name=${name}&variant=${variant}&size=${requestedSize}&set=facebook_icons&density=${density}x`;
+    return buildIconURL(name, requestedSize, density);
   },
 };