Cached url should be shared across watermark and sprite-selector-item for...

Cached url should be shared across watermark and sprite-selector-item for performance, address other review feedback.

src/containers/sprite-selector-item.jsx

@@ -46,15 +46,7 @@ class SpriteSelectorItem extends React.Component {
         if (this.props.costumeURL) return this.props.costumeURL;
         if (!this.props.assetId) return null;
 
-        if (this.decodedAssetId === this.props.assetId) {
-            // @todo consider caching more than one URL.
-            return this.cachedUrl;
-        }
-
-        this.decodedAssetId = this.props.assetId;
-        this.cachedUrl = getCostumeUrl(this.props.assetId, this.props.vm);
-
-        return this.cachedUrl;
+        return getCostumeUrl(this.props.assetId, this.props.vm);
     }
     handleMouseUp () {
         this.initialOffset = null;

src/containers/watermark.jsx

 import bindAll from 'lodash.bindall';
+import omit from 'lodash.omit';
 import PropTypes from 'prop-types';
 import React from 'react';
 import {connect} from 'react-redux';
@@ -21,28 +22,15 @@ class Watermark extends React.Component {
     getCostumeData () {
         if (!this.props.assetId) return null;
 
-        if (this.decodedAssetId === this.props.assetId) {
-            return this.cachedUrl;
-        }
-
-        this.decodedAssetId = this.props.assetId;
-        this.cachedUrl = getCostumeUrl(this.props.assetId, this.props.vm);
-
-        return this.cachedUrl;
+        return getCostumeUrl(this.props.assetId, this.props.vm);
     }
 
     render () {
-        const {
-            /* eslint-disable no-unused-vars */
-            assetId,
-            vm,
-            /* eslint-enable no-unused-vars */
-            ...props
-        } = this.props;
+        const componentProps = omit(this.props, ['assetId', 'vm']);
         return (
             <WatermarkComponent
                 costumeURL={this.getCostumeData()}
-                {...props}
+                {...componentProps}
             />
         );
     }
@@ -73,11 +61,8 @@ const mapStateToProps = state => {
     };
 };
 
-const mapDispatchToProps = () => ({});
-
 const ConnectedComponent = connect(
-    mapStateToProps,
-    mapDispatchToProps
+    mapStateToProps
 )(Watermark);
 
 export default ConnectedComponent;

src/lib/get-costume-url.js

@@ -3,27 +3,37 @@ import {SVGRenderer} from 'scratch-svg-renderer';
 // Contains 'font-family', but doesn't only contain 'font-family="none"'
 const HAS_FONT_REGEXP = 'font-family(?!="none")';
 
-const getCostumeUrl = function (assetId, vm) {
-
-    const storage = vm.runtime.storage;
-    const asset = storage.get(assetId);
-    // If the SVG refers to fonts, they must be inlined in order to display correctly in the img tag.
-    // Avoid parsing the SVG when possible, since it's expensive.
-    if (asset.assetType === storage.AssetType.ImageVector) {
-        // If the asset ID has not changed, no need to re-parse
-
-        const svgRenderer = new SVGRenderer();
-
-        const svgString = vm.runtime.storage.get(assetId).decodeText();
-        if (svgString.match(HAS_FONT_REGEXP)) {
-            svgRenderer.loadString(svgString);
-            const svgText = svgRenderer.toString(true /* shouldInjectFonts */);
-            return `data:image/svg+xml;utf8,${encodeURIComponent(svgText)}`;
+const getCostumeUrl = (function () {
+    let cachedAssetId;
+    let cachedUrl;
+
+    return function (assetId, vm) {
+
+        if (cachedAssetId === assetId) {
+            return cachedUrl;
         }
-        return vm.runtime.storage.get(assetId).encodeDataURI();
-    }
-    return vm.runtime.storage.get(assetId).encodeDataURI();
-};
+
+        cachedAssetId = assetId;
+
+        const storage = vm.runtime.storage;
+        const asset = storage.get(assetId);
+        // If the SVG refers to fonts, they must be inlined in order to display correctly in the img tag.
+        // Avoid parsing the SVG when possible, since it's expensive.
+        if (asset.assetType === storage.AssetType.ImageVector) {
+            const svgString = vm.runtime.storage.get(assetId).decodeText();
+            if (svgString.match(HAS_FONT_REGEXP)) {
+                const svgRenderer = new SVGRenderer();
+                svgRenderer.loadString(svgString);
+                const svgText = svgRenderer.toString(true /* shouldInjectFonts */);
+                cachedUrl = `data:image/svg+xml;utf8,${encodeURIComponent(svgText)}`;
+            }
+            cachedUrl = vm.runtime.storage.get(assetId).encodeDataURI();
+        }
+        cachedUrl = vm.runtime.storage.get(assetId).encodeDataURI();
+
+        return cachedUrl;
+    };
+}());
 
 export {
     getCostumeUrl as default,