fix(sketch): resolve icon sync artwork issues

packages/sketch/src/commands/icons/shared.js

@@ -14,13 +14,12 @@ import { syncSymbol } from '../../tools/symbols';
 
 const metadata = require('../../../generated/icons/metadata.json');
 
-export function syncIconSymbols(
+export function syncIconSymbols({
   document,
   symbols,
   symbolsPage,
-  sharedLayerStyles,
-  sizes = [32, 24, 20, 16]
-) {
+  sizes = [32, 24, 20, 16],
+}) {
   const sharedStyles = syncColorStyles(document, 'fill');
   const [sharedStyle] = sharedStyles.filter(
     ({ name }) => name === 'color / fill / black'
@@ -40,12 +39,16 @@ export function syncIconSymbols(
   );
 
   return artboards.map((artboard) => {
-    return syncSymbol(symbols, sharedLayerStyles, artboard.name, {
+    return syncSymbol({
+      symbols,
       name: artboard.name,
-      frame: artboard.frame,
-      layers: artboard.layers,
-      background: artboard.background,
-      parent: symbolsPage,
+      config: {
+        name: artboard.name,
+        frame: artboard.frame,
+        layers: artboard.layers,
+        background: artboard.background,
+        parent: symbolsPage,
+      },
     });
   });
 }

packages/sketch/src/commands/icons/sync.js

@@ -15,13 +15,12 @@ export function syncSmallIcons() {
     const document = Document.getSelectedDocument();
     const symbolsPage = findOrCreateSymbolPage(document);
     const symbols = document.getSymbols();
-    syncIconSymbols(
+    syncIconSymbols({
       document,
-      Array.from(symbols),
+      symbols: Array.from(symbols),
       symbolsPage,
-      document.sharedLayerStyles,
-      [16, 20]
-    );
+      sizes: [16, 20],
+    });
   });
 }
 
@@ -30,12 +29,11 @@ export function syncLargeIcons() {
     const document = Document.getSelectedDocument();
     const symbolsPage = findOrCreateSymbolPage(document);
     const symbols = document.getSymbols();
-    syncIconSymbols(
+    syncIconSymbols({
       document,
-      Array.from(symbols),
+      symbols: Array.from(symbols),
       symbolsPage,
-      document.sharedLayerStyles,
-      [24, 32]
-    );
+      sizes: [24, 32],
+    });
   });
 }

packages/sketch/src/commands/test/sync-symbol-id.js

@@ -26,19 +26,18 @@ export function testSyncSymbolId() {
   command('commands/test/sync-symbol-id', () => {
     const document = Document.getSelectedDocument();
 
-    syncSymbol(
-      document.getSymbols(),
-      document.sharedLayerStyles,
-      'test-symbol',
-      {
+    syncSymbol({
+      symbols: document.getSymbols(),
+      name: 'test-symbol',
+      config: {
         layers: [
           new ShapePath({
             name: 'Inner',
             shapeType: ShapePath.ShapeType.Oval,
             frame: new Rectangle(0, 0, 16, 16),
           }),
         ],
-      }
-    );
+      },
+    });
   });
 }

packages/sketch/src/tools/symbols.js

@@ -10,13 +10,14 @@ import { SymbolMaster } from 'sketch/dom';
 /**
  * Sync the given symbol name with a corresponding config in a document. Will
  * return a new symbol if none exist with the given name.
- * @param {Array<SymbolMaster>} symbols
- * @param {Array<SharedLayerStyles>} sharedLayerStyles
- * @param {string} name - the name of the symbol
- * @param {object} config - the config for the corresponding symbol master
+ * @param {object} params - syncSymbol parameters
+ * @param {Array<SymbolMaster>} params.symbols
+ * @param {string} params.name - the name of the symbol
+ * @param {object} params.config - the config for the corresponding symbol
+ * master
  * @returns {SketchSymbol}
  */
-export function syncSymbol(symbols, sharedLayerStyles, name, config) {
+export function syncSymbol({ symbols, name, config }) {
   const symbol = symbols.find((symbol) => symbol.name === name);
 
   if (!symbol) {
@@ -27,100 +28,8 @@ export function syncSymbol(symbols, sharedLayerStyles, name, config) {
   }
 
   Object.keys(config).forEach((key) => {
-    if (key === 'layers') {
-      syncSymbolLayers(sharedLayerStyles, symbol, config);
-    } else {
-      symbol[key] = config[key];
-    }
+    symbol[key] = config[key];
   });
 
   return symbol;
 }
-
-/**
- * Sync the layers between the original sketch symbol master and a proposed
- * configuration for this symbol. We will try our best to update the layers for
- * the original symbol, but may have to de-opt and use the proposed layers if we
- * cannot safely merge the two.
- * @param {Array<SharedLayerStyle>} sharedLayerStyles
- * @param {SketchSymbol} original - the original sketch symbol master
- * @param {object} proposed - a proposed set of changes to the symbol
- * @returns {void}
- */
-function syncSymbolLayers(sharedLayerStyles, original, proposed) {
-  // We're going to update the `layers` value on `original` to what is found on
-  // `proposed`. If we can, we'll update the `original` layer in-place. In cases
-  // where we cannot, we'll use the proposed layers.
-  //
-  // The preference is to keep the original layer in-place, namely for symbol
-  // overrides that may exist in the document. If we remove the layer and update
-  // it with a proposed layer, then we lose that override information meaning
-  // that designers will have to go through and update their symbols again
-  // instead of it automatically being updated.
-  //
-  // If nothing is shared between original and proposed, then the layers in
-  // proposed will be used.
-  original.layers = proposed.layers.map((proposedLayer) => {
-    // We often name nested layers that can be overridden.
-    const { name } = proposedLayer;
-    const originalLayer = original.layers.find((layer) => {
-      return layer.name === name;
-    });
-
-    // If we can find a layer in `proposed` with a corresponding `original`
-    // layer, then we'll attempt to merge the two.
-    if (originalLayer) {
-      // We can't safely merge if the type or shapeType have diverged so we'll
-      // need to default to the changed layer
-      if (
-        originalLayer.type !== proposedLayer.type ||
-        originalLayer.shapeType !== proposedLayer.shapeType
-      ) {
-        return proposedLayer;
-      }
-
-      // Attempt to merge the two layers
-      merge(originalLayer, proposedLayer);
-
-      // If our original layer has a shared style, we'll update it to match
-      if (originalLayer.sharedStyleId) {
-        const sharedStyle = sharedLayerStyles.find((sharedStyle) => {
-          return originalLayer.sharedStyleId === sharedStyle.id;
-        });
-
-        if (sharedStyle) {
-          const opacity = originalLayer.style.opacity;
-          originalLayer.style.syncWithSharedStyle(sharedStyle);
-          // We'll want to keep things like opacity from being overridden by the
-          // shared style as we use it to hide layers in certain icons
-          originalLayer.style.opacity = opacity;
-        }
-      }
-
-      return originalLayer;
-    }
-
-    // By default, we'll defer to the proposed layer
-    return proposedLayer;
-  });
-}
-
-const defaultPropertyList = new Set(['frame', 'points', 'style']);
-
-/**
- * Merge a given source layer with a target layer. Optionally specify a
- * propertyList that will specify what keys are safe to merge
- *
- * @param {object} source - the source layer that we are trying to update
- * in-place
- * @param {object} target - the target layer that contains the latest changes
- * @param {?Set} propertyList - specify which properties are safe to merge
- * @returns {void}
- */
-function merge(source, target, propertyList = defaultPropertyList) {
-  for (let key in target) {
-    if (propertyList.has(key)) {
-      source[key] = target[key];
-    }
-  }
-}