fix: duplicate loaders when using src import with loader options

close #1278

lib/loaders/pitcher.js

@@ -17,13 +17,26 @@ module.exports.pitch = function (remainingRequest) {
     return
   }
 
-  // loader.request contains both the resolved loader path and its options
-  // query (e.g. ??ref-0)
-  const toLoaderString = loader => loader.request
+  const genRequest = loaders => {
+    // Important: dedupe since both the original rule
+    // and the cloned rule would match a source import request.
+    // also make sure to dedupe based on loader path.
+    // assumes you'd probably never want to apply the same loader on the same
+    // file twice.
+    const seen = new Map()
+    const loaderStrings = []
+
+    loaders.forEach(loader => {
+      const type = typeof loader === 'string' ? loader : loader.path
+      const request = typeof loader === 'string' ? loader : loader.request
+      if (!seen.has(type)) {
+        seen.set(type, true)
+        // loader.request contains both the resolved loader path and its options
+        // query (e.g. ??ref-0)
+        loaderStrings.push(request)
+      }
+    })
 
-  const genRequest = loaderStrings => {
-    // important: dedupe
-    loaderStrings = Array.from(new Set(loaderStrings))
     return loaderUtils.stringifyRequest(this, '-!' + [
       ...loaderStrings,
       this.resourcePath + this.resourceQuery
@@ -34,8 +47,8 @@ module.exports.pitch = function (remainingRequest) {
   if (query.type === `style`) {
     const cssLoaderIndex = loaders.findIndex(l => /(\/|\\)css-loader/.test(l.path))
     if (cssLoaderIndex) {
-      const afterLoaders = loaders.slice(0, cssLoaderIndex + 1).map(toLoaderString)
-      const beforeLoaders = loaders.slice(cssLoaderIndex + 1).map(toLoaderString)
+      const afterLoaders = loaders.slice(0, cssLoaderIndex + 1)
+      const beforeLoaders = loaders.slice(cssLoaderIndex + 1)
       const request = genRequest([
         ...afterLoaders,
         stylePostLoaderPath,
@@ -48,10 +61,9 @@ module.exports.pitch = function (remainingRequest) {
 
   // for templates: inject the template compiler
   if (query.type === `template`) {
-    const beforeLoaders = loaders.map(toLoaderString)
     const request = genRequest([
       templateLoaderPath + `??vue-loader-options`,
-      ...beforeLoaders
+      ...loaders
     ])
     // console.log(request)
     // the template compiler uses esm exports
@@ -69,6 +81,6 @@ module.exports.pitch = function (remainingRequest) {
   // When the user defines a rule that has only resourceQuery but no test,
   // both that rule and the cloned rule will match, resulting in duplicated
   // loaders. Therefore it is necessary to perform a dedupe here.
-  const request = genRequest(loaders.map(toLoaderString))
+  const request = genRequest(loaders)
   return `import mod from ${request}; export default mod; export * from ${request}`
 }

package.json

@@ -70,6 +70,8 @@
     "stylus": "^0.54.5",
     "stylus-loader": "^3.0.2",
     "sugarss": "^1.0.1",
+    "ts-loader": "^4.2.0",
+    "typescript": "^2.8.3",
     "url-loader": "^1.0.1",
     "vue": "^2.5.16",
     "vue-server-renderer": "^2.5.16",

test/edgeCases.spec.js

@@ -10,6 +10,10 @@ const {
 } = require('./utils')
 
 const assertComponent = ({ window, module }, done) => {
+  if (typeof module === 'function') {
+    module = module.options
+  }
+
   const vnode = mockRender(module, {
     msg: 'hi'
   })
@@ -162,3 +166,21 @@ test('usage with null-loader', done => {
     done()
   })
 })
+
+test('proper dedupe on src-imports with options', done => {
+  mockBundleAndRun({
+    entry: 'ts.vue',
+    resolve: {
+      extensions: ['.ts', '.js']
+    },
+    module: {
+      rules: [
+        {
+          test: /\.ts$/,
+          loader: 'ts-loader',
+          options: { appendTsSuffixTo: [/\.vue$/] }
+        }
+      ]
+    }
+  }, res => assertComponent(res, done))
+})

test/fixtures/App.ts

+import Vue from 'vue'
+
+export default Vue.extend({
+  data () {
+    return {
+      msg: 'Hello from Component A!'
+    }
+  },
+  methods: {
+    someMethod (arg: string): string {
+      return 'hello'
+    }
+  }
+})

test/fixtures/ts.vue

+<template>
+  <h2 class="red">{{ msg }}</h2>
+</template>
+
+ <script src="./App.ts" lang="ts"></script>
+
+<style>
+comp-a h2 {
+  color: #f00;
+}
+</style>

tsconfig.json

+{
+  "compilerOptions": {
+    "module": "esnext",
+    "moduleResolution": "node",
+    "strict": true,
+    "target": "es6",
+    "baseUrl": "."
+  },
+  "exclude": [
+    "node_modules"
+  ]
+}

yarn.lock

@@ -8293,7 +8293,7 @@ semver-diff@^2.0.0:
   dependencies:
     semver "^5.0.3"
 
-"semver@2 || 3 || 4 || 5", semver@^5.0.3, semver@^5.1.0, semver@^5.3.0, semver@^5.4.1, semver@^5.5.0:
+"semver@2 || 3 || 4 || 5", semver@^5.0.1, semver@^5.0.3, semver@^5.1.0, semver@^5.3.0, semver@^5.4.1, semver@^5.5.0:
   version "5.5.0"
   resolved "https://registry.yarnpkg.com/semver/-/semver-5.5.0.tgz#dc4bbc7a6ca9d916dee5d43516f0092b58f7b8ab"
 
@@ -9164,6 +9164,16 @@ trim-right@^1.0.1:
   dependencies:
     glob "^6.0.4"
 
+ts-loader@^4.2.0:
+  version "4.2.0"
+  resolved "https://registry.yarnpkg.com/ts-loader/-/ts-loader-4.2.0.tgz#c380c399fc81f82cad0e3044f9c1f775ecde6efa"
+  dependencies:
+    chalk "^2.3.0"
+    enhanced-resolve "^4.0.0"
+    loader-utils "^1.0.2"
+    micromatch "^3.1.4"
+    semver "^5.0.1"
+
 tty-browserify@0.0.0:
   version "0.0.0"
   resolved "https://registry.yarnpkg.com/tty-browserify/-/tty-browserify-0.0.0.tgz#a157ba402da24e9bf957f9aa69d524eed42901a6"
@@ -9199,6 +9209,10 @@ typedarray@^0.0.6:
   version "0.0.6"
   resolved "https://registry.yarnpkg.com/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777"
 
+typescript@^2.8.3:
+  version "2.8.3"
+  resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.8.3.tgz#5d817f9b6f31bb871835f4edf0089f21abe6c170"
+
 uc.micro@^1.0.1, uc.micro@^1.0.5:
   version "1.0.5"
   resolved "https://registry.yarnpkg.com/uc.micro/-/uc.micro-1.0.5.tgz#0c65f15f815aa08b560a61ce8b4db7ffc3f45376"