fix: Fix sourcemaps when using cy.origin() dependencies (#24367)

npm/webpack-preprocessor/lib/cross-origin-callback-loader.ts

@@ -3,6 +3,7 @@ import { parse } from '@babel/parser'
 import { default as traverse } from '@babel/traverse'
 import { default as generate } from '@babel/generator'
 import { NodePath, types as t } from '@babel/core'
+import mergeSourceMaps from 'merge-source-map'
 import * as loaderUtils from 'loader-utils'
 import * as pathUtil from 'path'
 import Debug from 'debug'
@@ -41,6 +42,7 @@ export default function (source: string, map, meta, store = crossOriginCallbackS
       allowSuperOutsideMethod: true,
       allowUndeclaredExports: true,
       sourceType: 'unambiguous',
+      sourceFilename: resourcePath,
     })
   } catch (err) {
     // it's unlikely there will be a parsing error, since that should have
@@ -155,22 +157,24 @@ export default function (source: string, map, meta, store = crossOriginCallbackS
     },
   })
 
-  // if we found requires/imports, re-generate the code from the AST
-  if (hasDependencies) {
-    debug('callback with modified source')
-
-    // TODO: handle sourcemaps for this correctly
-    // https://github.com/cypress-io/cypress/issues/23365
-    // the following causes error "Cannot read property 'replace' of undefined"
-    //   return generate(ast, { sourceMaps: true }, source).code
-    // and can't pass in original map or the output ends up with
-    // `undefinedundefined` appended, which is a syntax error
-    this.callback(null, generate(ast, {}).code)
+  // if no requires/imports were found, callback with the original source/map
+  if (!hasDependencies) {
+    debug('callback with original source')
+    this.callback(null, source, map)
 
     return
   }
 
-  debug('callback with original source')
-  // if no requires/imports were found, callback with the original source/map
-  this.callback(null, source, map)
+  // if we found requires/imports, re-generate the code from the AST
+  const result = generate(ast, { sourceMaps: true }, {
+    [resourcePath]: source,
+  })
+  // result.map needs to be merged with the original map for it to include
+  // the changes made in this loader. we can't return result.map because it
+  // is based off the intermediary code provided to the loader and not the
+  // original source code (which could be TypeScript or JSX or something)
+  const newMap = mergeSourceMaps(map, result.map)
+
+  debug('callback with modified source')
+  this.callback(null, result.code, newMap)
 }

npm/webpack-preprocessor/package.json

@@ -8,6 +8,7 @@
     "build": "rimraf dist && tsc || echo 'built, with errors'",
     "build-prod": "yarn build",
     "deps": "deps-ok && dependency-check --no-dev .",
+    "postinstall": "patch-package",
     "secure": "nsp check",
     "semantic-release": "semantic-release",
     "size": "npm pack --dry",
@@ -26,6 +27,8 @@
     "loader-utils": "^2.0.0",
     "lodash": "^4.17.20",
     "md5": "2.3.0",
+    "merge-source-map": "1.1.0",
+    "patch-package": "6.4.7",
     "webpack-virtual-modules": "^0.4.4"
   },
   "devDependencies": {
@@ -87,5 +90,10 @@
     "cypress-plugin",
     "cypress-preprocessor",
     "webpack"
-  ]
+  ],
+  "workspaces": {
+    "nohoist": [
+      "merge-source-map"
+    ]
+  }
 }

npm/webpack-preprocessor/patches/merge-source-map+1.1.0.patch

@@ -0,0 +1,14 @@
+diff --git a/node_modules/merge-source-map/index.js b/node_modules/merge-source-map/index.js
+index 2867fb7..efa44ed 100644
+--- a/node_modules/merge-source-map/index.js
++++ b/node_modules/merge-source-map/index.js
+@@ -47,7 +47,8 @@ function merge(oldMap, newMap) {
+     })
+   })
+
+-  var consumers = [oldMapConsumer, newMapConsumer]
++  // fixes https://github.com/keik/merge-source-map/issues/6
++  var consumers = [newMapConsumer, oldMapConsumer]
+   consumers.forEach(function(consumer) {
+     consumer.sources.forEach(function(sourceFile) {
+       mergedMapGenerator._sources.add(sourceFile)

npm/webpack-preprocessor/test/unit/cross-origin-callback-loader.spec.ts

@@ -24,7 +24,12 @@ describe('./lib/cross-origin-callback-loader', () => {
       resourcePath: '/path/to/file',
       query: { commands },
     }
-    const originalMap = { sourcesContent: [] }
+    const originalMap = {
+      sources: [],
+      sourcesContent: [],
+      version: 3,
+      mappings: [],
+    }
 
     store.addFile = sinon.stub()
     loader.call(context, source, originalMap, null, store)
@@ -91,12 +96,13 @@ describe('./lib/cross-origin-callback-loader', () => {
     })
 
     it('replaces cy.origin() callback with an object when using require()', () => {
-      const { resultingSource, resultingMap } = callLoader(stripIndent`
+      const source = stripIndent`
         it('test', () => {
           cy.origin('http://www.foobar.com:3500', () => {
             require('../support/utils')
           })
-        })`)
+        })`
+      const { originalMap, resultingSource, resultingMap } = callLoader(source)
 
       expect(resultingSource).to.equal(stripIndent`
         it('test', () => {
@@ -106,16 +112,19 @@ describe('./lib/cross-origin-callback-loader', () => {
           });
         });`)
 
-      expect(resultingMap).to.be.undefined
+      expect(resultingMap).to.exist
+      expect(resultingMap).not.to.equal(originalMap)
+      expect(resultingMap.sourcesContent[0]).to.equal(source)
     })
 
     it('replaces cy.origin() callback with an object when using import()', () => {
-      const { resultingSource, resultingMap } = callLoader(stripIndent`
+      const source = stripIndent`
         it('test', () => {
           cy.origin('http://www.foobar.com:3500', async () => {
             await import('../support/utils')
           })
-        })`)
+        })`
+      const { originalMap, resultingSource, resultingMap } = callLoader(source)
 
       expect(resultingSource).to.equal(stripIndent`
         it('test', () => {
@@ -125,17 +134,19 @@ describe('./lib/cross-origin-callback-loader', () => {
           });
         });`)
 
-      expect(resultingMap).to.be.undefined
+      expect(resultingMap).to.exist
+      expect(resultingMap).not.to.equal(originalMap)
+      expect(resultingMap.sourcesContent[0]).to.equal(source)
     })
 
     it('replaces cy.other() when specified in commands', () => {
-      const { resultingSource, resultingMap } = callLoader(stripIndent`
+      const source = stripIndent`
         it('test', () => {
           cy.other('http://www.foobar.com:3500', () => {
             require('../support/utils')
           })
-        })`,
-      ['other'])
+        })`
+      const { originalMap, resultingSource, resultingMap } = callLoader(source, ['other'])
 
       expect(resultingSource).to.equal(stripIndent`
         it('test', () => {
@@ -145,7 +156,9 @@ describe('./lib/cross-origin-callback-loader', () => {
           });
         });`)
 
-      expect(resultingMap).to.be.undefined
+      expect(resultingMap).to.exist
+      expect(resultingMap).not.to.equal(originalMap)
+      expect(resultingMap.sourcesContent[0]).to.equal(source)
     })
 
     it('adds the file to the store, replacing require() with require()', () => {
@@ -217,7 +230,9 @@ describe('./lib/cross-origin-callback-loader', () => {
         `it('test', () => {
           cy.origin('http://www.foobar.com:3500', () => {
             require('../support/commands')
+
             const utils = require('../support/utils')
+
             const _ = require('lodash')
           })
         })`,
@@ -255,7 +270,9 @@ describe('./lib/cross-origin-callback-loader', () => {
         `it('test', () => {
           cy.origin('http://www.foobar.com:3500', () => {
             const someVar = 'someValue'
+
             const result = require('./fn')(someVar)
+
             expect(result).to.equal('mutated someVar')
           })
         })`,
@@ -276,6 +293,7 @@ describe('./lib/cross-origin-callback-loader', () => {
         `it('test', () => {
           cy.origin('http://www.foobar.com:3500', { args: { foo: 'foo'}}, ({ foo }) => {
             const result = require('./fn')(foo)
+
             expect(result).to.equal('mutated someVar')
           })
         })`,

package.json

@@ -255,6 +255,7 @@
     ],
     "nohoist": [
       "**/webpack-preprocessor/babel-loader",
+      "**/webpack-preprocessor/**/merge-source-map",
       "**/webpack-batteries-included-preprocessor/ts-loader",
       "**/jest",
       "**/jest*",

system-tests/projects/e2e/cypress/e2e/cy_origin_error.cy.ts

@@ -1,14 +1,48 @@
-describe('cy.origin', () => {
+import { fail, verify } from '../support/util'
+
+describe('cy.origin errors', () => {
   beforeEach(() => {
+    // @ts-ignore
+    window.top.__cySkipValidateConfig = true
+    // must be interactive or stack trace is handled differently for the sake
+    // of how it prints to stdout
+    Cypress.config('isInteractive', true)
+
     cy.visit('/primary_origin.html')
     cy.get('a[data-cy="cross_origin_secondary_link"]').click()
   })
 
-  it('tries to find an element that doesn\'t exist and fails', () => {
+  fail('command failure', this, () => {
     cy.origin('http://www.foobar.com:4466', () => {
-      cy.get('#doesnotexist', {
-        timeout: 1000,
-      })
+      cy.get('#doesnotexist', { timeout: 1 })
     })
   })
+
+  verify('command failure', this, {
+    line: 16,
+    column: 8,
+    message: 'Expected to find element',
+    stack: ['cy_origin_error.cy.ts'],
+    before () {
+      cy.visit('/primary_origin.html')
+    },
+  })
+
+  fail('failure when using dependency', this, () => {
+    cy.origin('http://www.foobar.com:4466', () => {
+      require('../support/util')
+
+      cy.get('#doesnotexist', { timeout: 1 })
+    })
+  })
+
+  verify('failure when using dependency', this, {
+    line: 32,
+    column: 8,
+    message: 'Expected to find element',
+    stack: ['cy_origin_error.cy.ts'],
+    before () {
+      cy.visit('/primary_origin.html')
+    },
+  })
 })