From b916ba9c410856582b113fcdcef69cfc6b6e9861 Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Fri, 28 Oct 2022 17:51:11 -0400 Subject: [PATCH] fix: Fix sourcemaps when using cy.origin() dependencies (#24367) --- .../lib/cross-origin-callback-loader.ts | 32 ++++---- npm/webpack-preprocessor/package.json | 10 ++- .../patches/merge-source-map+1.1.0.patch | 14 ++++ .../unit/cross-origin-callback-loader.spec.ts | 40 +++++++--- package.json | 1 + .../__snapshots__/cy_origin_error_spec.ts.js | 75 ------------------- .../e2e/cypress/e2e/cy_origin_error.cy.ts | 44 +++++++++-- .../projects/e2e/cypress/support/util.js | 23 +++--- .../e2e/failing.cy.js | 4 +- .../cypress/e2e/failing.cy.ts | 4 +- .../cypress/e2e/failing.cy.ts | 4 +- .../cypress/e2e/failing.cy.ts | 4 +- .../cypress/e2e/failing.cy.js | 4 +- system-tests/test/cy_origin_error_spec.ts | 14 ++-- yarn.lock | 7 ++ 15 files changed, 147 insertions(+), 133 deletions(-) create mode 100644 npm/webpack-preprocessor/patches/merge-source-map+1.1.0.patch delete mode 100644 system-tests/__snapshots__/cy_origin_error_spec.ts.js diff --git a/npm/webpack-preprocessor/lib/cross-origin-callback-loader.ts b/npm/webpack-preprocessor/lib/cross-origin-callback-loader.ts index 3845bcf253c8..32fc084036ab 100644 --- a/npm/webpack-preprocessor/lib/cross-origin-callback-loader.ts +++ b/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) } diff --git a/npm/webpack-preprocessor/package.json b/npm/webpack-preprocessor/package.json index 386dcd5877a0..1d0dd66af9b7 100644 --- a/npm/webpack-preprocessor/package.json +++ b/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" + ] + } } diff --git a/npm/webpack-preprocessor/patches/merge-source-map+1.1.0.patch b/npm/webpack-preprocessor/patches/merge-source-map+1.1.0.patch new file mode 100644 index 000000000000..51c60ecf1633 --- /dev/null +++ b/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) diff --git a/npm/webpack-preprocessor/test/unit/cross-origin-callback-loader.spec.ts b/npm/webpack-preprocessor/test/unit/cross-origin-callback-loader.spec.ts index 98714050e181..0e390956bc5f 100644 --- a/npm/webpack-preprocessor/test/unit/cross-origin-callback-loader.spec.ts +++ b/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') }) })`, diff --git a/package.json b/package.json index 4ce2769a4444..33888fe147d3 100644 --- a/package.json +++ b/package.json @@ -255,6 +255,7 @@ ], "nohoist": [ "**/webpack-preprocessor/babel-loader", + "**/webpack-preprocessor/**/merge-source-map", "**/webpack-batteries-included-preprocessor/ts-loader", "**/jest", "**/jest*", diff --git a/system-tests/__snapshots__/cy_origin_error_spec.ts.js b/system-tests/__snapshots__/cy_origin_error_spec.ts.js deleted file mode 100644 index db957e5e5a90..000000000000 --- a/system-tests/__snapshots__/cy_origin_error_spec.ts.js +++ /dev/null @@ -1,75 +0,0 @@ -exports['e2e cy.origin errors / captures the stack trace correctly for errors in cross origins to point users to their "cy.origin" callback'] = ` - -==================================================================================================== - - (Run Starting) - - ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ Cypress: 1.2.3 │ - │ Browser: FooBrowser 88 │ - │ Specs: 1 found (cy_origin_error.cy.ts) │ - │ Searched: cypress/e2e/cy_origin_error.cy.ts │ - │ Experiments: experimentalSessionAndOrigin=true │ - └────────────────────────────────────────────────────────────────────────────────────────────────┘ - - -──────────────────────────────────────────────────────────────────────────────────────────────────── - - Running: cy_origin_error.cy.ts (1 of 1) - - - cy.origin - 1) tries to find an element that doesn't exist and fails - - - 0 passing - 1 failing - - 1) cy.origin - tries to find an element that doesn't exist and fails: - AssertionError: Timed out retrying after 1000ms: Expected to find element: \`#doesnotexist\`, but never found it. - [stack trace lines] - - - - - (Results) - - ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ Tests: 1 │ - │ Passing: 0 │ - │ Failing: 1 │ - │ Pending: 0 │ - │ Skipped: 0 │ - │ Screenshots: 1 │ - │ Video: true │ - │ Duration: X seconds │ - │ Spec Ran: cy_origin_error.cy.ts │ - └────────────────────────────────────────────────────────────────────────────────────────────────┘ - - - (Screenshots) - - - /XXX/XXX/XXX/cypress/screenshots/cy_origin_error.cy.ts/cy.origin -- tries to fin (1280x720) - d an element that doesn't exist and fails (failed).png - - - (Video) - - - Started processing: Compressing to 32 CRF - - Finished processing: /XXX/XXX/XXX/cypress/videos/cy_origin_error.cy.ts.mp4 (X second) - - -==================================================================================================== - - (Run Finished) - - - Spec Tests Passing Failing Pending Skipped - ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ ✖ cy_origin_error.cy.ts XX:XX 1 - 1 - - │ - └────────────────────────────────────────────────────────────────────────────────────────────────┘ - ✖ 1 of 1 failed (100%) XX:XX 1 - 1 - - - - -` diff --git a/system-tests/projects/e2e/cypress/e2e/cy_origin_error.cy.ts b/system-tests/projects/e2e/cypress/e2e/cy_origin_error.cy.ts index e3dda2e01045..9b0658599ba0 100644 --- a/system-tests/projects/e2e/cypress/e2e/cy_origin_error.cy.ts +++ b/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') + }, + }) }) diff --git a/system-tests/projects/e2e/cypress/support/util.js b/system-tests/projects/e2e/cypress/support/util.js index cbff55bc2b68..b53912d9a26f 100644 --- a/system-tests/projects/e2e/cypress/support/util.js +++ b/system-tests/projects/e2e/cypress/support/util.js @@ -5,20 +5,21 @@ let openInIdePath = Cypress.spec // ensure title is unique since it's necessary for querying the UI // in the verification step -const getTitle = (ctx) => { +const getTitle = (title, ctx) => { const parentTitle = ctx.parent && ctx.parent.title - return `${parentTitle} ${ctx.title}`.trim() + return `${parentTitle} ${title}`.trim() } -export const fail = (ctx, test) => { - const title = `${count++}) ✗ FAIL - ${getTitle(ctx)}` +export const fail = (title, ctx, test) => { + const testTitle = `${count++}) ✗ FAIL - ${getTitle(title, ctx)}` - it(title, { defaultCommandTimeout: 0 }, test) + it(testTitle, test) } -export const verify = (ctx, options) => { +export const verify = (title, ctx, options) => { const { + before, line, column, message, @@ -28,10 +29,12 @@ export const verify = (ctx, options) => { const codeFrameFileRegex = new RegExp(`${Cypress.spec.relative}:${line}:${column}`) const stackFileRegex = new RegExp(`${Cypress.spec.relative}:${line}:${column - 1}`) - it(`✓ VERIFY`, function () { + it(`✓ VERIFY - ${title}`, function () { + if (before) before() + cy.wrap(Cypress.$(window.top.document.body)) .find('.reporter') - .contains(`FAIL - ${getTitle(ctx)}`) + .contains(`FAIL - ${getTitle(title, ctx)}`) .closest('.collapsible') .within(() => { cy.contains('View stack trace').click() @@ -63,8 +66,8 @@ export const verify = (ctx, options) => { .invoke('text') .should('match', codeFrameFileRegex) - // code frames will show `fail(this,()=>` as the 1st line - cy.get('.test-err-code-frame pre span').should('include.text', 'fail(this,()=>') + // code frames will show this as the 1st line + cy.get('.test-err-code-frame pre span').should('include.text', `fail('${title}',this,()=>`) cy.contains('.test-err-code-frame .runnable-err-file-path', openInIdePath.relative) }) diff --git a/system-tests/projects/integration-outside-project-root/e2e/failing.cy.js b/system-tests/projects/integration-outside-project-root/e2e/failing.cy.js index a15c054443cc..680691388875 100644 --- a/system-tests/projects/integration-outside-project-root/e2e/failing.cy.js +++ b/system-tests/projects/integration-outside-project-root/e2e/failing.cy.js @@ -13,11 +13,11 @@ context('validation errors', function () { Cypress.config('isInteractive', true) }) - fail(this, () => { + fail('validation error', this, () => { cy.viewport() }) - verify(this, { + verify('validation error', this, { line: 17, column: 8, message: 'can only accept a string preset or', diff --git a/system-tests/projects/webpack-preprocessor-awesome-typescript-loader/cypress/e2e/failing.cy.ts b/system-tests/projects/webpack-preprocessor-awesome-typescript-loader/cypress/e2e/failing.cy.ts index 8bad16995d92..e8b22c2a0d74 100644 --- a/system-tests/projects/webpack-preprocessor-awesome-typescript-loader/cypress/e2e/failing.cy.ts +++ b/system-tests/projects/webpack-preprocessor-awesome-typescript-loader/cypress/e2e/failing.cy.ts @@ -22,12 +22,12 @@ context('validation errors', function () { Cypress.config('isInteractive', true) }) - fail(this, () => { + fail('validation error', this, () => { // @ts-ignore cy.viewport() }) - verify(this, { + verify('validation error', this, { line: 27, column: 8, message: 'can only accept a string preset or', diff --git a/system-tests/projects/webpack-preprocessor-ts-loader-compiler-options/cypress/e2e/failing.cy.ts b/system-tests/projects/webpack-preprocessor-ts-loader-compiler-options/cypress/e2e/failing.cy.ts index 3447e9cabafe..8db1d0d37d78 100644 --- a/system-tests/projects/webpack-preprocessor-ts-loader-compiler-options/cypress/e2e/failing.cy.ts +++ b/system-tests/projects/webpack-preprocessor-ts-loader-compiler-options/cypress/e2e/failing.cy.ts @@ -21,12 +21,12 @@ context('validation errors', function () { Cypress.config('isInteractive', true) }) - fail(this, () => { + fail('validation error', this, () => { // @ts-ignore cy.viewport() }) - verify(this, { + verify('validation error', this, { line: 26, column: 8, message: 'can only accept a string preset or', diff --git a/system-tests/projects/webpack-preprocessor-ts-loader/cypress/e2e/failing.cy.ts b/system-tests/projects/webpack-preprocessor-ts-loader/cypress/e2e/failing.cy.ts index 3447e9cabafe..8db1d0d37d78 100644 --- a/system-tests/projects/webpack-preprocessor-ts-loader/cypress/e2e/failing.cy.ts +++ b/system-tests/projects/webpack-preprocessor-ts-loader/cypress/e2e/failing.cy.ts @@ -21,12 +21,12 @@ context('validation errors', function () { Cypress.config('isInteractive', true) }) - fail(this, () => { + fail('validation error', this, () => { // @ts-ignore cy.viewport() }) - verify(this, { + verify('validation error', this, { line: 26, column: 8, message: 'can only accept a string preset or', diff --git a/system-tests/projects/webpack-preprocessor/cypress/e2e/failing.cy.js b/system-tests/projects/webpack-preprocessor/cypress/e2e/failing.cy.js index 76c0b0975c79..3dfa89fd88ea 100644 --- a/system-tests/projects/webpack-preprocessor/cypress/e2e/failing.cy.js +++ b/system-tests/projects/webpack-preprocessor/cypress/e2e/failing.cy.js @@ -14,11 +14,11 @@ context('validation errors', function () { Cypress.config('isInteractive', true) }) - fail(this, () => { + fail('validation error', this, () => { cy.viewport() }) - verify(this, { + verify('validation error', this, { line: 18, column: 8, message: 'can only accept a string preset or', diff --git a/system-tests/test/cy_origin_error_spec.ts b/system-tests/test/cy_origin_error_spec.ts index 6a28a9e04b00..c7fc444b705e 100644 --- a/system-tests/test/cy_origin_error_spec.ts +++ b/system-tests/test/cy_origin_error_spec.ts @@ -32,17 +32,17 @@ describe('e2e cy.origin errors', () => { // keep the port the same to prevent issues with the snapshot port: PORT, spec: 'cy_origin_error.cy.ts', - snapshot: true, - expectedExitCode: 1, + expectedExitCode: 2, config: commonConfig, async onRun (exec) { - const res = await exec() + const { stdout } = await exec() - expect(res.stdout).to.contain('AssertionError') - expect(res.stdout).to.contain('Timed out retrying after 1000ms: Expected to find element: `#doesnotexist`, but never found it.') + expect(stdout).to.contain('AssertionError') + expect(stdout).to.contain('Timed out retrying after 1ms: Expected to find element: `#doesnotexist`, but never found it.') - // check to make sure the snapshot contains the 'cy.origin' sourcemap - expect(res.stdout).to.contain('webpack:///./cypress/e2e/cy_origin_error.cy.ts:8:7') + // check to make sure stack trace contains the 'cy.origin' source + expect(stdout).to.contain('webpack:///./cypress/e2e/cy_origin_error.cy.ts:16:7') + expect(stdout).to.contain('webpack:///./cypress/e2e/cy_origin_error.cy.ts:32:7') }, }) diff --git a/yarn.lock b/yarn.lock index bd6a21966228..6a5683d2ec57 100644 --- a/yarn.lock +++ b/yarn.lock @@ -23320,6 +23320,13 @@ merge-descriptors@1.0.1, merge-descriptors@~1.0.0: resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.1.tgz#b00aaa556dd8b44568150ec9d1b953f3f90cbb61" integrity sha1-sAqqVW3YtEVoFQ7J0blT8/kMu2E= +merge-source-map@1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/merge-source-map/-/merge-source-map-1.1.0.tgz#2fdde7e6020939f70906a68f2d7ae685e4c8c646" + integrity sha512-Qkcp7P2ygktpMPh2mCQZaf3jhN6D3Z/qVZHSdWvQ+2Ef5HgRAPBO57A77+ENm0CPx2+1Ce/MYKi3ymqdfuqibw== + dependencies: + source-map "^0.6.1" + merge-stream@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/merge-stream/-/merge-stream-2.0.0.tgz#52823629a14dd00c9770fb6ad47dc6310f2c1f60"