Move offline icon storage to app/

Summary:
This diff splits Facebook management into a ui-core and electron part:

* Cleaned up code and introduces a uniform Icon type to describe a requested icon
* Computing icon urls is done in the ui-core
* Introduced a RenderHost hook that can transform the request icon into a different url, in this case, a url to load icons from disk in production builds

For the browser UI, the urls are currently no rewritten since we have only dev builds (which always used only FB urls already). We could do the same rewrite in the future and download the static assets during build time. But for now this means that in the browser implementation we depend on normal browser caching, with the biggest downside that icons might not appear if the user has no internet connections.

With this change we lost our last usage of staticPath computations in ui-core

Reviewed By: aigoncharov

Differential Revision: D32767426

fbshipit-source-id: d573b6a373e649c7dacd380cf63a50c2dbbd9e70

desktop/app/src/electron/initializeElectron.tsx

@@ -25,7 +25,8 @@ import {
 import fs from 'fs';
 import {setupMenuBar} from './setupMenuBar';
 import {FlipperServer, FlipperServerConfig} from 'flipper-common';
-import type {RenderHost} from 'flipper-ui-core';
+import type {Icon, RenderHost} from 'flipper-ui-core';
+import {getLocalIconUrl} from '../utils/icons';
 
 export function initializeElectron(
   flipperServer: FlipperServer,
@@ -186,6 +187,14 @@ export function initializeElectron(
         path.resolve(flipperServerConfig.paths.staticPath, relativePath)
       );
     },
+    getLocalIconUrl(icon: Icon, url: string): string {
+      return getLocalIconUrl(
+        icon,
+        url,
+        flipperServerConfig.paths.appPath,
+        !flipperServerConfig.environmentInfo.isProduction,
+      );
+    },
   } as RenderHost;
 
   setupMenuBar();

desktop/app/src/utils/__tests__/icons.node.tsx

@@ -0,0 +1,65 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * 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, getLocalIconUrl} from '../icons';
+// eslint-disable-next-line flipper/no-relative-imports-across-packages
+import {getPublicIconUrl} from '../../../../flipper-ui-core/src/utils/icons';
+import * as path from 'path';
+import {getRenderHostInstance} from 'flipper-ui-core';
+import fs from 'fs';
+
+test('filled icons get correct local path', () => {
+  const iconPath = buildLocalIconPath({
+    name: 'star',
+    variant: 'filled',
+    size: 12,
+    density: 2,
+  });
+  expect(iconPath).toBe(path.join('icons', 'star-filled-12@2x.png'));
+});
+
+test('outline icons get correct local path', () => {
+  const iconPath = buildLocalIconPath({
+    name: 'star',
+    variant: 'outline',
+    size: 12,
+    density: 2,
+  });
+  expect(iconPath).toBe(path.join('icons', 'star-outline-12@2x.png'));
+});
+
+test('filled icons get correct URL', async () => {
+  const icon = {
+    name: 'star',
+    variant: 'filled',
+    size: 12,
+    density: 2,
+  } as const;
+  const iconUrl = getPublicIconUrl(icon);
+  expect(iconUrl).toBe(
+    'https://facebook.com/assets/?name=star&variant=filled&size=12&set=facebook_icons&density=2x',
+  );
+  const staticPath = getRenderHostInstance().serverConfig.paths.staticPath;
+  const localUrl = getLocalIconUrl(icon, iconUrl, staticPath, false);
+  // since files don't exist at disk in de checkouts
+  expect(localUrl).toBe(iconUrl);
+
+  // ... let's mock a file
+  const iconPath = path.join(staticPath, 'icons', 'star-filled-12@2x.png');
+  try {
+    await fs.promises.writeFile(
+      iconPath,
+      'Generated for unit tests. Please remove',
+    );
+    // should now generate a absolute path
+    expect(getLocalIconUrl(icon, iconUrl, staticPath, false)).toBe(iconPath);
+  } finally {
+    await fs.promises.unlink(iconPath);
+  }
+});

desktop/app/src/utils/icons.tsx

@@ -0,0 +1,92 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ */
+
+// We should get rid of sync use entirely but until then the
+// methods are marked as such.
+/* eslint-disable node/no-sync */
+
+import fs from 'fs';
+import path from 'path';
+import type {Icon} from 'flipper-ui-core';
+
+export type Icons = {
+  [key: string]: Icon['size'][];
+};
+
+let _icons: Icons | undefined;
+
+function getIconsSync(staticPath: string): Icons {
+  return (
+    _icons! ??
+    (_icons = JSON.parse(
+      fs.readFileSync(path.join(staticPath, 'icons.json'), {encoding: 'utf8'}),
+    ))
+  );
+}
+
+export function buildLocalIconPath(icon: Icon) {
+  return path.join(
+    'icons',
+    `${icon.name}-${icon.variant}-${icon.size}@${icon.density}x.png`,
+  );
+}
+
+export function getLocalIconUrl(
+  icon: Icon,
+  url: string,
+  basePath: string,
+  registerIcon: boolean,
+): string {
+  // resolve icon locally if possible
+  const iconPath = path.join(basePath, buildLocalIconPath(icon));
+  if (fs.existsSync(iconPath)) {
+    return iconPath;
+  }
+  if (registerIcon) {
+    tryRegisterIcon(icon, url, basePath);
+  }
+
+  return url; // fall back to http URL
+}
+
+function tryRegisterIcon(icon: Icon, url: string, staticPath: string) {
+  const entryName = icon.name + (icon.variant === 'outline' ? '-outline' : '');
+  const {size} = icon;
+  const icons = getIconsSync(staticPath);
+  if (!icons[entryName]?.includes(size)) {
+    const existing = icons[entryName] || (icons[entryName] = []);
+    if (!existing.includes(size)) {
+      // Check if that icon actually exists!
+      fetch(url)
+        .then((res) => {
+          if (res.status !== 200) {
+            throw new Error(
+              // eslint-disable-next-line prettier/prettier
+              `Trying to use icon '${entryName}' with size ${size} and density ${icon.density}, however the icon doesn't seem to exists at ${url}: ${res.status}`,
+            );
+          }
+          if (!existing.includes(size)) {
+            // the icon exists
+            existing.push(size);
+            existing.sort();
+            fs.writeFileSync(
+              path.join(staticPath, 'icons.json'),
+              JSON.stringify(icons, null, 2),
+              'utf8',
+            );
+            console.warn(
+              `Added uncached icon "${entryName}: [${size}]" to /static/icons.json.`,
+            );
+          } else {
+          }
+        })
+        .catch((e) => console.error(e));
+    }
+  }
+}

desktop/flipper-ui-core/src/RenderHost.tsx

@@ -12,6 +12,8 @@ import type {PluginNotification} from './reducers/notifications';
 import type {NotificationConstructorOptions} from 'electron';
 import {FlipperLib} from 'flipper-plugin';
 import {FlipperServer, FlipperServerConfig} from 'flipper-common';
+import {IconSize} from './ui/components/Glyph';
+import {Icon} from './utils/icons';
 
 // Events that are emitted from the main.ts ovr the IPC process bridge in Electron
 type MainProcessEvents = {
@@ -98,6 +100,8 @@ export interface RenderHost {
   serverConfig: FlipperServerConfig;
   requirePlugin(path: string): Promise<any>;
   getStaticResourceUrl(relativePath: string): string;
+  // given the requested icon and proposed public url of the icon, rewrite it to a local icon if needed
+  getLocalIconUrl?(icon: Icon, publicUrl: string): string;
 }
 
 export function getRenderHostInstance(): RenderHost {

desktop/flipper-ui-core/src/deprecated-exports.tsx

@@ -91,7 +91,6 @@ export {default as StatusIndicator} from './ui/components/StatusIndicator';
 export {default as HorizontalRule} from './ui/components/HorizontalRule';
 export {default as Label} from './ui/components/Label';
 export {default as Heading} from './ui/components/Heading';
-export * from './utils/pathUtils';
 export {Filter} from './ui/components/filter/types';
 export {default as StackTrace} from './ui/components/StackTrace';
 export {

desktop/flipper-ui-core/src/index.tsx

@@ -13,3 +13,5 @@ export * from './deprecated-exports';
 export {RenderHost, getRenderHostInstance} from './RenderHost';
 
 export {startFlipperDesktop} from './startFlipperDesktop';
+
+export {Icon} from './utils/icons';

desktop/flipper-ui-core/src/ui/components/Glyph.tsx

@@ -9,7 +9,7 @@
 
 import React from 'react';
 import styled from '@emotion/styled';
-import {getIconURLSync} from '../../utils/icons';
+import {getIconURL} from '../../utils/icons';
 
 export type IconSize = 8 | 10 | 12 | 16 | 18 | 20 | 24 | 32;
 
@@ -121,11 +121,12 @@ export default class Glyph extends React.PureComponent<{
         color={color}
         size={size}
         title={title}
-        src={getIconURLSync(
+        src={getIconURL({
-          variant === 'outline' ? `${name}-outline` : name,
+          name,
+          variant: variant ?? 'filled',
           size,
-          typeof window !== 'undefined' ? window.devicePixelRatio : 1,
+          density: typeof window !== 'undefined' ? window.devicePixelRatio : 1,
-        )}
+        })}
         style={style}
       />
     );

desktop/flipper-ui-core/src/utils/__tests__/icons.node.tsx

@@ -1,35 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * 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, buildIconURLSync} from '../icons';
-import * as path from 'path';
-
-test('filled icons get correct local path', () => {
-  const iconPath = buildLocalIconPath('star', 12, 2);
-  expect(iconPath).toBe(path.join('icons', 'star-filled-12@2x.png'));
-});
-
-test('outline icons get correct local path', () => {
-  const iconPath = buildLocalIconPath('star-outline', 12, 2);
-  expect(iconPath).toBe(path.join('icons', 'star-outline-12@2x.png'));
-});
-
-test('filled icons get correct URL', () => {
-  const iconUrl = buildIconURLSync('star', 12, 2);
-  expect(iconUrl).toBe(
-    'https://facebook.com/assets/?name=star&variant=filled&size=12&set=facebook_icons&density=2x',
-  );
-});
-
-test('outline icons get correct URL', () => {
-  const iconUrl = buildIconURLSync('star-outline', 12, 2);
-  expect(iconUrl).toBe(
-    'https://facebook.com/assets/?name=star&variant=outline&size=12&set=facebook_icons&density=2x',
-  );
-});

desktop/flipper-ui-core/src/utils/icons.d.ts

@@ -1,20 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- * @format
- */
-
-import {IconSize} from '../ui/components/Glyph';
-
-declare function getIconURL(
-  name: string,
-  size?: IconSize,
-  density?: number,
-): string;
-
-declare const ICONS: {
-  [key: string]: Array<IconSize>;
-};

desktop/flipper-ui-core/src/utils/icons.ts

@@ -1,167 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- * @format
- */
-
-// We should get rid of sync use entirely but until then the
-// methods are marked as such.
-/* eslint-disable node/no-sync */
-
-import fs from 'fs';
-import path from 'path';
-import {getRenderHostInstance} from '../RenderHost';
-import {getStaticPath} from './pathUtils';
-
-const AVAILABLE_SIZES = [8, 10, 12, 16, 18, 20, 24, 32];
-const DENSITIES = [1, 1.5, 2, 3, 4];
-
-function getIconsPath() {
-  return getStaticPath('icons.json');
-}
-
-export type Icons = {
-  [key: string]: number[];
-};
-
-let _icons: Icons | undefined;
-
-export function getIconsSync(): Icons {
-  return (
-    _icons! ??
-    (_icons = JSON.parse(fs.readFileSync(getIconsPath(), {encoding: 'utf8'})))
-  );
-}
-
-// Takes a string like 'star', or 'star-outline', and converts it to
-// {trimmedName: 'star', variant: 'filled'} or {trimmedName: 'star', variant: 'outline'}
-function getIconPartsFromName(icon: string): {
-  trimmedName: string;
-  variant: 'outline' | 'filled';
-} {
-  const isOutlineVersion = icon.endsWith('-outline');
-  const trimmedName = isOutlineVersion ? icon.replace('-outline', '') : icon;
-  const variant = isOutlineVersion ? 'outline' : 'filled';
-  return {trimmedName: trimmedName, variant: variant};
-}
-
-function getIconFileName(
-  icon: {trimmedName: string; variant: 'outline' | 'filled'},
-  size: number,
-  density: number,
-) {
-  return `${icon.trimmedName}-${icon.variant}-${size}@${density}x.png`;
-}
-
-export function buildLocalIconPath(
-  name: string,
-  size: number,
-  density: number,
-) {
-  const icon = getIconPartsFromName(name);
-  return path.join('icons', getIconFileName(icon, size, density));
-}
-
-export function buildLocalIconURL(name: string, size: number, density: number) {
-  const icon = getIconPartsFromName(name);
-  return `icons/${getIconFileName(icon, size, density)}`;
-}
-
-export function buildIconURLSync(name: string, size: number, density: number) {
-  const icon = getIconPartsFromName(name);
-  // eslint-disable-next-line prettier/prettier
-  const url = `https://facebook.com/assets/?name=${icon.trimmedName}&variant=${icon.variant}&size=${size}&set=facebook_icons&density=${density}x`;
-  if (
-    typeof window !== 'undefined' &&
-    (!getIconsSync()[name] || !getIconsSync()[name].includes(size))
-  ) {
-    // From utils/isProduction
-    const isProduction = !/node_modules[\\/]electron[\\/]/.test(
-      getRenderHostInstance().serverConfig.paths.execPath,
-    );
-
-    if (!isProduction) {
-      const existing = getIconsSync()[name] || (getIconsSync()[name] = []);
-      if (!existing.includes(size)) {
-        // Check if that icon actually exists!
-        fetch(url)
-          .then((res) => {
-            if (res.status === 200 && !existing.includes(size)) {
-              // the icon exists
-              existing.push(size);
-              existing.sort();
-              fs.writeFileSync(
-                getIconsPath(),
-                JSON.stringify(getIconsSync(), null, 2),
-                'utf8',
-              );
-              console.warn(
-                `Added uncached icon "${name}: [${size}]" to /static/icons.json. Restart Flipper to apply the change.`,
-              );
-            } else {
-              throw new Error(
-                // eslint-disable-next-line prettier/prettier
-                `Trying to use icon '${name}' with size ${size} and density ${density}, however the icon doesn't seem to exists at ${url}: ${res.status}`,
-              );
-            }
-          })
-          .catch((e) => console.error(e));
-      }
-    } else {
-      console.warn(
-        `Using uncached icon: "${name}: [${size}]". Add it to /static/icons.json to preload it.`,
-      );
-    }
-  }
-  return url;
-}
-
-export function getIconURLSync(
-  name: string,
-  size: number,
-  density: number,
-  basePath: string = getRenderHostInstance().serverConfig.paths.appPath,
-) {
-  if (name.indexOf('/') > -1) {
-    return name;
-  }
-
-  let requestedSize = size;
-  if (!AVAILABLE_SIZES.includes(size)) {
-    // find the next largest size
-    const possibleSize = AVAILABLE_SIZES.find((size) => {
-      return size > requestedSize;
-    });
-
-    // set to largest size if the real size is larger than what we have
-    if (possibleSize == null) {
-      requestedSize = Math.max(...AVAILABLE_SIZES);
-    } else {
-      requestedSize = possibleSize;
-    }
-  }
-
-  if (!DENSITIES.includes(density)) {
-    // find the next largest size
-    const possibleDensity = DENSITIES.find((scale) => {
-      return scale > density;
-    });
-
-    // set to largest size if the real size is larger than what we have
-    if (possibleDensity == null) {
-      density = Math.max(...DENSITIES);
-    } else {
-      density = possibleDensity;
-    }
-  }
-
-  // resolve icon locally if possible
-  const iconPath = path.join(basePath, buildLocalIconPath(name, size, density));
-  if (fs.existsSync(iconPath)) {
-    return buildLocalIconURL(name, size, density);
-  }
-  return buildIconURLSync(name, requestedSize, density);
-}

desktop/flipper-ui-core/src/utils/icons.tsx

@@ -0,0 +1,74 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ */
+
+import {getRenderHostInstance} from '../RenderHost';
+import {IconSize} from '../ui/components/Glyph';
+
+const AVAILABLE_SIZES: IconSize[] = [8, 10, 12, 16, 18, 20, 24, 32];
+const DENSITIES = [1, 1.5, 2, 3, 4];
+
+export type Icon = {
+  name: string;
+  variant: 'outline' | 'filled';
+  size: IconSize;
+  density: number;
+};
+
+function normalizeIcon(icon: Icon): Icon {
+  let {size, density} = icon;
+  let requestedSize = size as number;
+  if (!AVAILABLE_SIZES.includes(size as any)) {
+    // find the next largest size
+    const possibleSize = AVAILABLE_SIZES.find((size) => {
+      return size > requestedSize;
+    });
+
+    // set to largest size if the real size is larger than what we have
+    if (possibleSize == null) {
+      requestedSize = Math.max(...AVAILABLE_SIZES);
+    } else {
+      requestedSize = possibleSize;
+    }
+  }
+
+  if (!DENSITIES.includes(density)) {
+    // find the next largest size
+    const possibleDensity = DENSITIES.find((scale) => {
+      return scale > density;
+    });
+
+    // set to largest size if the real size is larger than what we have
+    if (possibleDensity == null) {
+      density = Math.max(...DENSITIES);
+    } else {
+      density = possibleDensity;
+    }
+  }
+
+  return {
+    ...icon,
+    size: requestedSize as IconSize,
+    density,
+  };
+}
+
+export function getPublicIconUrl({name, variant, size, density}: Icon) {
+  return `https://facebook.com/assets/?name=${name}&variant=${variant}&size=${size}&set=facebook_icons&density=${density}x`;
+}
+
+export function getIconURL(icon: Icon) {
+  if (icon.name.indexOf('/') > -1) {
+    return icon.name;
+  }
+
+  icon = normalizeIcon(icon);
+  const baseUrl = getPublicIconUrl(icon);
+
+  return getRenderHostInstance().getLocalIconUrl?.(icon, baseUrl) ?? baseUrl;
+}

desktop/flipper-ui-core/src/utils/pathUtils.tsx

@@ -1,32 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- * @format
- */
-
-// We use sync access once per startup.
-/* eslint-disable node/no-sync */
-
-import path from 'path';
-import {getRenderHostInstance} from '../RenderHost';
-
-/**
- * @deprecated
- */
-export function getStaticPath(
-  relativePath: string = '.',
-  {asarUnpacked}: {asarUnpacked: boolean} = {asarUnpacked: false},
-) {
-  const staticDir = getRenderHostInstance().serverConfig.paths.staticPath;
-  const absolutePath = path.resolve(staticDir, relativePath);
-  // Unfortunately, path.resolve, fs.pathExists, fs.read etc do not automatically work with asarUnpacked files.
-  // All these functions still look for files in "app.asar" even if they are unpacked.
-  // Looks like automatic resolving for asarUnpacked files only work for "child_process" module.
-  // So we're using a hack here to actually look to "app.asar.unpacked" dir instead of app.asar package.
-  return asarUnpacked
-    ? absolutePath.replace('app.asar', 'app.asar.unpacked')
-    : absolutePath;
-}

desktop/scripts/build-release.ts

@@ -29,17 +29,13 @@ import {
   moveSourceMaps,
 } from './build-utils';
 import fetch from '@adobe/node-fetch-retry';
-import {
+import {buildLocalIconPath, Icons} from '../app/src/utils/icons';
-  getIconsSync,
-  buildLocalIconPath,
-  getIconURLSync,
-  Icons,
-} from '../flipper-ui-core/src/utils/icons';
 import isFB from './isFB';
 import copyPackageWithDependencies from './copy-package-with-dependencies';
 import {staticDir, distDir} from './paths';
 import yargs from 'yargs';
 import {WinPackager} from 'app-builder-lib/out/winPackager';
+import {Icon, getPublicIconUrl} from 'flipper-ui-core/src/utils/icons';
 
 // Used in some places to avoid release-to-release changes. Needs
 // to be this high for some MacOS-specific things that I can't
@@ -309,30 +305,40 @@ async function copyStaticFolder(buildFolder: string) {
   console.log('✅  Copied static package with dependencies.');
 }
 
-function downloadIcons(buildFolder: string) {
+// Takes a string like 'star', or 'star-outline', and converts it to
+// {trimmedName: 'star', variant: 'filled'} or {trimmedName: 'star', variant: 'outline'}
+function getIconPartsFromName(icon: string): {
+  trimmedName: string;
+  variant: 'outline' | 'filled';
+} {
+  const isOutlineVersion = icon.endsWith('-outline');
+  const trimmedName = isOutlineVersion ? icon.replace('-outline', '') : icon;
+  const variant = isOutlineVersion ? 'outline' : 'filled';
+  return {trimmedName: trimmedName, variant: variant};
+}
+
+async function downloadIcons(buildFolder: string) {
   const icons: Icons = JSON.parse(
-    fs.readFileSync(path.join(buildFolder, 'icons.json'), {
+    await fs.promises.readFile(path.join(buildFolder, 'icons.json'), {
       encoding: 'utf8',
     }),
   );
-  const iconURLs = Object.entries(icons).reduce<
+  const iconURLs = Object.entries(icons).reduce<Icon[]>(
-    {
+    (acc, [entryName, sizes]) => {
-      name: string;
+      const {trimmedName: name, variant} = getIconPartsFromName(entryName);
-      size: number;
+      acc.push(
-      density: number;
+        // get icons in @1x and @2x
-    }[]
+        ...sizes.map((size) => ({name, variant, size, density: 1})),
-  >((acc, [name, sizes]) => {
+        ...sizes.map((size) => ({name, variant, size, density: 2})),
-    acc.push(
+      );
-      // get icons in @1x and @2x
+      return acc;
-      ...sizes.map((size) => ({name, size, density: 1})),
+    },
-      ...sizes.map((size) => ({name, size, density: 2})),
+    [],
-    );
+  );
-    return acc;
-  }, []);
 
   return Promise.all(
-    iconURLs.map(({name, size, density}) => {
+    iconURLs.map((icon) => {
-      const url = getIconURLSync(name, size, density, buildFolder);
+      const url = getPublicIconUrl(icon);
       return fetch(url, {
         retryOptions: {
           // Be default, only 5xx are retried but we're getting the odd 404
@@ -344,9 +350,7 @@ function downloadIcons(buildFolder: string) {
           if (res.status !== 200) {
             throw new Error(
               // eslint-disable-next-line prettier/prettier
-              `Could not download the icon ${name} from ${url}: got status ${
+              `Could not download the icon ${name} from ${url}: got status ${res.status}`,
-                res.status
-              }`,
             );
           }
           return res;
@@ -355,7 +359,7 @@ function downloadIcons(buildFolder: string) {
           (res) =>
             new Promise((resolve, reject) => {
               const fileStream = fs.createWriteStream(
-                path.join(buildFolder, buildLocalIconPath(name, size, density)),
+                path.join(buildFolder, buildLocalIconPath(icon)),
               );
               res.body.pipe(fileStream);
               res.body.on('error', reject);