fix: smart merge should maintain existing loader order

In a situation where loader order is specified in a
previous config, one of those loaders being referenced in
a subsequent config shouldn't ignore the previous order.

Fixes #79

CHANGELOG.md

@@ -1,3 +1,8 @@
+4.1.3 / 2018-06-04
+==================
+
+  * Fix - Smart merge respects the existing loader order #79
+
 4.1.1 / 2017-11-01
 ==================
 

README.md

@@ -271,6 +271,69 @@ merge.smart({
 }
 ```
 
+This also works in reverse - the existing order will be maintained if possible:
+
+```javascript
+merge.smart({
+  loaders: [{
+    test: /\.css$/,
+    use: [
+      { loader: 'css-loader', options: { myOptions: true } },
+      { loader: 'style-loader' }
+    ]
+  }]
+}, {
+  loaders: [{
+    test: /\.css$/,
+    use: [
+      { loader: 'style-loader', options: { someSetting: true } }
+    ]
+  }]
+});
+// will become
+{
+  loaders: [{
+    test: /\.css$/,
+    use: [
+      { loader: 'css-loader', options: { myOptions: true } },
+      { loader: 'style-loader', options: { someSetting: true } }
+    ]
+  }]
+}
+```
+
+In the case of an order conflict, the second order wins:
+```javascript
+merge.smart({
+  loaders: [{
+    test: /\.css$/,
+    use: [
+      { loader: 'css-loader' },
+      { loader: 'style-loader' }
+    ]
+  }]
+}, {
+  loaders: [{
+    test: /\.css$/,
+    use: [
+      { loader: 'style-loader' },
+      { loader: 'css-loader' }
+    ]
+  }]
+});
+// will become
+{
+  loaders: [{
+    test: /\.css$/,
+    use: [
+      { loader: 'style-loader' }
+      { loader: 'css-loader' },
+    ]
+  }]
+}
+```
+
+
 **Loader query strings `loaders: ['babel?plugins[]=object-assign']` will be overridden.**
 
 ```javascript

src/join-arrays-smart.js

@@ -1,5 +1,5 @@
 import {
-  isEqual, mergeWith, unionWith, differenceWith
+  isEqual, mergeWith, differenceWith
 } from 'lodash';
 
 const isArray = Array.isArray;
@@ -72,13 +72,7 @@ function uniteRules(rules, key, newRule, rule) {
         rule[loadersKey] = newRule.use || newRule.loaders;
         break;
       default:
-        rule[loadersKey] = unionWith(
-          // Remove existing entries so that we can respect the order of the new
-          // entries
-          differenceWith(entries, newEntries, isEqual),
-          newEntries,
-          uniteEntries
-        ).map(unwrapEntry);
+        rule[loadersKey] = combineEntries(newEntries, entries).map(unwrapEntry);
     }
   }
 
@@ -106,19 +100,106 @@ function isSameValue(a, b) {
   return isEqual(propA, propB);
 }
 
-function uniteEntries(newEntry, entry) {
+function areEqualEntries(newEntry, entry) {
   const loaderNameRe = /^([^?]+)/ig;
 
   const [loaderName] = entry.loader.match(loaderNameRe);
   const [newLoaderName] = newEntry.loader.match(loaderNameRe);
 
-  if (loaderName !== newLoaderName) {
-    return false;
+  return loaderName === newLoaderName;
+}
+
+function uniteEntries(newEntry, entry) {
+  if (areEqualEntries(newEntry, entry)) {
+    // Replace query values with newer ones
+    mergeWith(entry, newEntry);
+    return true;
   }
+  return false;
+}
 
-  // Replace query values with newer ones
-  mergeWith(entry, newEntry);
-  return true;
+/* Combines entries and newEntries, while respecting the order of loaders in each.
+
+Iterates through new entries. If the new entry also exists in existing entries,
+we'll put in all of the loaders from existing entries that come before it (in case
+those are pre-requisites). Any remaining existing entries are added at the end.
+
+Since webpack processes right-to-left, we're working backwards through the arrays
+*/
+function combineEntries(newEntries, existingEntries) {
+  const resultSet = [];
+
+  // We're iterating through newEntries, this keeps track of where we are in the existingEntries
+  let existingEntriesIteratorIndex = existingEntries.length - 1;
+
+  for (let i = newEntries.length - 1; i >= 0; i -= 1) {
+    const currentEntry = newEntries[i];
+    const indexInExistingEntries =
+      findLastIndexUsingComparinator(
+        existingEntries,
+        currentEntry,
+        areEqualEntries,
+        existingEntriesIteratorIndex);
+    const hasEquivalentEntryInExistingEntries = indexInExistingEntries !== -1;
+
+    if (hasEquivalentEntryInExistingEntries) {
+      // If the same entry exists in existing entries, we should add all of the entries that
+      // come before to maintain order
+      for (let j = existingEntriesIteratorIndex; j > indexInExistingEntries; j -= 1) {
+        const existingEntry = existingEntries[j];
+
+        // If this entry also exists in new entries, we'll add as part of iterating through
+        // new entries so that if there's a conflict between existing entries and new entries,
+        // new entries order wins
+        const hasMatchingEntryInNewEntries =
+          findLastIndexUsingComparinator(newEntries, existingEntry, areEqualEntries, i) !== -1;
+
+        if (!hasMatchingEntryInNewEntries) {
+          resultSet.unshift(existingEntry);
+        }
+        existingEntriesIteratorIndex -= 1;
+      }
+
+      uniteEntries(currentEntry, existingEntries[existingEntriesIteratorIndex]);
+      // uniteEntries mutates the second parameter to be a merged version, so that's what's pushed
+      resultSet.unshift(existingEntries[existingEntriesIteratorIndex]);
+
+      existingEntriesIteratorIndex -= 1;
+    } else {
+      const alreadyHasMatchingEntryInResultSet =
+        findLastIndexUsingComparinator(resultSet, currentEntry, areEqualEntries) !== -1;
+
+      if (!alreadyHasMatchingEntryInResultSet) {
+        resultSet.unshift(currentEntry);
+      }
+    }
+
+  }
+
+  // Add remaining existing entries
+  for (existingEntriesIteratorIndex; existingEntriesIteratorIndex >= 0;
+    existingEntriesIteratorIndex -= 1) {
+
+    const existingEntry = existingEntries[existingEntriesIteratorIndex];
+    const alreadyHasMatchingEntryInResultSet =
+      findLastIndexUsingComparinator(resultSet, existingEntry, areEqualEntries) !== -1;
+
+    if (!alreadyHasMatchingEntryInResultSet) {
+      resultSet.unshift(existingEntry);
+    }
+  }
+
+  return resultSet;
+}
+
+function findLastIndexUsingComparinator(entries, entryToFind, comparinator, startingIndex) {
+  startingIndex = startingIndex || entries.length - 1;
+  for (let i = startingIndex; i >= 0; i -= 1) {
+    if (areEqualEntries(entryToFind, entries[i])) {
+      return i;
+    }
+  }
+  return -1;
 }
 
 export {

tests/merge-smart-tests.js

@@ -843,6 +843,90 @@ function mergeSmartTest(merge, loadersKey) {
 
     assert.deepEqual(merge(common, extractText), result);
   });
+
+  it('should respect the existing order for ' + loadersKey, function () {
+    const common = {
+      module: {}
+    };
+    common.module[loadersKey] = [
+      {
+        test: /\.css$/,
+        use: [
+          { loader: 'css-loader', options: { myOptions: true } },
+          { loader: 'style-loader' }
+        ]
+      }
+    ];
+    const extractText = {
+      module: {}
+    };
+    extractText.module[loadersKey] = [
+      {
+        test: /\.css$/,
+        use: [
+          { loader: 'style-loader', options: { someSetting: true } }
+        ]
+      }
+    ];
+    const result = {
+      module: {}
+    };
+    result.module[loadersKey] = [
+      {
+        test: /\.css$/,
+        use: [
+          { loader: 'css-loader', options: { myOptions: true } },
+          { loader: 'style-loader', options: { someSetting: true } }
+        ]
+      }
+    ];
+
+    assert.deepEqual(merge(common, extractText), result);
+  });
+
+  it('should respect second order when existing/new have conflicting orders for ' + loadersKey, function () {
+    const common = {
+      module: {}
+    };
+    common.module[loadersKey] = [
+      {
+        test: /\.css$/,
+        use: [
+          { loader: 'css-loader' },
+          { loader: 'style-loader' },
+          { loader: 'other-loader' }
+        ]
+      }
+    ];
+
+    const extractText = {
+      module: {}
+    };
+    extractText.module[loadersKey] = [
+      {
+        test: /\.css$/,
+        use: [
+          { loader: 'style-loader' },
+          { loader: 'css-loader' }
+        ]
+      }
+    ];
+    const result = {
+      module: {}
+    };
+    result.module[loadersKey] = [
+      {
+        test: /\.css$/,
+        use: [
+          'style-loader',
+          'css-loader',
+          'other-loader'
+        ]
+      }
+    ];
+
+    assert.deepEqual(merge(common, extractText), result);
+  });
 }
 
 module.exports = mergeSmartTests;