Add fixes based on feedback

Shopify · May 14, 2018 · 6b0f590 · 6b0f590
1 parent 262e4e0
commit 6b0f590
Show file tree

Hide file tree

Showing 8 changed files with 27 additions and 43 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -1,3 +1,4 @@
+packages/slate-cssvar-loader/__tests__/fixtures
 packages/slate-theme/dist
 packages/slate-theme/src/scripts/**/*.js.liquid
 **/node_modules/

diff --git a/packages/concat-style-loader/concat.js b/packages/concat-style-loader/concat.js
@@ -3,11 +3,11 @@ const path = require('path');
 
 function concatStyles(content, rootPath, loader) {
   const assets = parseImports(content, rootPath, loader).reverse();
-  let inlinedAssets;
+  let inlinedAssets = content;
 
   assets.forEach((asset) => {
     asset.content = concatStyles(asset.content, asset.path);
-    inlinedAssets = inlineAsset(content, asset);
+    inlinedAssets = inlineAsset(inlinedAssets, asset);
   });
 
   return inlinedAssets;
@@ -27,9 +27,9 @@ function parseImports(content, rootPath, loader) {
     const url = getImportURL(match[0]);
     const assetPath = path.resolve(path.dirname(rootPath), url);
 
-    const modifiedContent = fetchAssetContent(assetPath, loader);
+    const importedContent = fetchAssetContent(assetPath, loader);
 
-    return {path: assetPath, modifiedContent, match};
+    return {path: assetPath, content: importedContent, match};
   });
 }
 

diff --git a/packages/create-slate-theme/utils.js b/packages/create-slate-theme/utils.js
@@ -9,17 +9,17 @@ function spawn(cmd, options = {stdio: 'inherit'}) {
 function splitCommandString(cmd) {
   const regexp = /[^\s"]+|"([^"]*)"/gi;
   const parts = [];
+  let match;
 
   do {
     // Each call to exec returns the next regex match as an array
-    const match = regexp.exec(cmd);
+    match = regexp.exec(cmd);
 
     if (match != null) {
       // Index 1 in the array is the captured group if it exists
       // Index 0 is the matched text, which we use if no captured group exists
       parts.push(match[1] ? match[1] : match[0]);
     }
-    /* eslint-disable-next-line no-undef */
   } while (match != null);
 
   return parts;

diff --git a/packages/slate-cssvar-loader/__tests__/fixtures/expected-disabled.js b/packages/slate-cssvar-loader/__tests__/fixtures/expected-disabled.js
@@ -1,13 +1,8 @@
-module.exports = require('../../node_modules/css-loader/lib/css-base.js')(
-  false,
-);
+exports = module.exports = require("../../node_modules/css-loader/lib/css-base.js")(false);
 // imports
 
+
 // module
-module.exports.push([
-  module.id,
-  ".heading {\n  color: var(--headings_color);\n  background-color: '#FF00FF';\n  letter-spacing: var(--letter_spacing);\n}\n\n.title {\n  color: var(--headings_color);\n  font-weight: var(--font-body-bold-weight);\n}\n\nbody {\n  background-color: var(--body_color);\n}",
-  '',
-]);
+exports.push([module.id, ".heading {\n  color: var(--headings_color);\n  background-color: '#FF00FF';\n  letter-spacing: var(--letter_spacing);\n}\n\n.title {\n  color: var(--headings_color);\n  font-weight: var(--font-body-bold-weight);\n}\n\nbody {\n  background-color: var(--body_color);\n}", ""]);
 
 // exports
diff --git a/packages/slate-cssvar-loader/__tests__/fixtures/expected-morevars.js b/packages/slate-cssvar-loader/__tests__/fixtures/expected-morevars.js
@@ -1,13 +1,8 @@
-module.exports = require('../../node_modules/css-loader/lib/css-base.js')(
-  false,
-);
+exports = module.exports = require("../../node_modules/css-loader/lib/css-base.js")(false);
 // imports
 
+
 // module
-module.exports.push([
-  module.id,
-  ".heading {\n  color: {{ settings.headings_color }};\n  background-color: '#FF00FF';\n}\n\n.title {\n  color: {{ settings.headings_color }};\n}\n\nbody {\n  background-color: {{ settings.body_color }};\n  color: {{ settings.extra_variable }};\n}",
-  '',
-]);
+exports.push([module.id, ".heading {\n  color: {{ settings.headings_color }};\n  background-color: '#FF00FF';\n}\n\n.title {\n  color: {{ settings.headings_color }};\n}\n\nbody {\n  background-color: {{ settings.body_color }};\n  color: {{ settings.extra_variable }};\n}", ""]);
 
 // exports
diff --git a/packages/slate-cssvar-loader/__tests__/fixtures/expected.js b/packages/slate-cssvar-loader/__tests__/fixtures/expected.js
@@ -1,13 +1,8 @@
-module.exports = require('../../node_modules/css-loader/lib/css-base.js')(
-  false,
-);
+exports = module.exports = require("../../node_modules/css-loader/lib/css-base.js")(false);
 // imports
 
+
 // module
-module.exports.push([
-  module.id,
-  '.heading {\n  color: {{ settings.headings_color }};\n  background-color: \'#FF00FF\';\n  letter-spacing: {{ settings.letter_spacing }}px;\n}\n\n.title {\n  color: {{ settings.headings_color }};\n  font-weight: {{ font_body_bold.weight | default: "bold" }};\n}\n\nbody {\n  background-color: {{ settings.body_color }};\n}',
-  '',
-]);
+exports.push([module.id, ".heading {\n  color: {{ settings.headings_color }};\n  background-color: '#FF00FF';\n  letter-spacing: {{ settings.letter_spacing }}px;\n}\n\n.title {\n  color: {{ settings.headings_color }};\n  font-weight: {{ font_body_bold.weight | default: \"bold\" }};\n}\n\nbody {\n  background-color: {{ settings.body_color }};\n}", ""]);
 
 // exports
diff --git a/packages/slate-tools/cli/commands/start.js b/packages/slate-tools/cli/commands/start.js
@@ -82,7 +82,8 @@ devServer.compiler.hooks.done.tap('CLI', (stats) => {
 
 devServer.client.hooks.beforeSync.tapPromise('CLI', async (files) => {
   if (firstSync && argv.skipFirstDeploy) {
-    return (devServer.skipDeploy = true);
+    devServer.skipDeploy = true;
+    return;
   }
 
   if (continueIfPublishedTheme === null) {
@@ -102,8 +103,6 @@ devServer.client.hooks.beforeSync.tapPromise('CLI', async (files) => {
       (file) => !file.endsWith('settings_data.json'),
     );
   }
-
-  return true;
 });
 
 devServer.client.hooks.syncSkipped.tap('CLI', () => {

diff --git a/packages/slate-tools/tools/webpack/get-chunk-name.js b/packages/slate-tools/tools/webpack/get-chunk-name.js
@@ -1,3 +1,5 @@
+const crypto = require('crypto');
+
 module.exports = function(module, chunks, cacheGroup) {
   let containsLayout = false;
   const names = chunks
@@ -24,20 +26,17 @@ module.exports = function(module, chunks, cacheGroup) {
   // been set to 100 to prevent `[name].[chunkhash].[ext]` from
   // generating a 256+ character string.
   if (name.length > 256) {
-    name = `${name.slice(0, 240)}~${hashString(name)}`;
+    name = `${name.slice(0, 240)}~${hashFilename(name)}`;
   }
 
   /* eslint-disable-next-line consistent-return */
   return name;
 };
 
-function hashString(text) {
-  let hash = 5381;
-  let index = text.length;
-
-  while (index) {
-    hash = (hash * 33) ^ text.charCodeAt(--index);
-  }
-
-  return hash >>> 0;
+function hashFilename(name) {
+  return crypto
+    .createHash('md4')
+    .update(name)
+    .digest('hex')
+    .slice(0, 8);
 }