diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 9e62d583b56c..46fe84b9139a 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -7,6 +7,10 @@ _Released 7/02/2024 (PENDING)_ - Fixed an issue where Chrome launch instances would not recreate the browser CRI client correctly after recovering from an unexpected browser closure. Fixes [#27657](https://github.com/cypress-io/cypress/issues/27657). Fixed in [#29663](https://github.com/cypress-io/cypress/pull/29663). +**Performance:** + +- Improved performance of `experimentalSourceRewriting` option. Fixed in [#29540](https://github.com/cypress-io/cypress/pull/29540). + **Bugfixes:** - Fixed an issue where Firefox 129 (Firefox Nightly) would not launch with Cypress. Fixes [#29713](https://github.com/cypress-io/cypress/issues/29713). Fixed in [#29720](https://github.com/cypress-io/cypress/pull/29720). diff --git a/packages/rewriter/lib/js-rules.ts b/packages/rewriter/lib/js-rules.ts index bc8d05e5fe88..ddd2fb369130 100644 --- a/packages/rewriter/lib/js-rules.ts +++ b/packages/rewriter/lib/js-rules.ts @@ -62,6 +62,8 @@ function resolveLocationReference () { ) } +const replaceableProps = ['parent', 'top', 'location'] + /** * Given an Identifier or a Literal, return a property name that should use `resolveWindowReference`. * @param node @@ -70,12 +72,12 @@ function getReplaceablePropOfMemberExpression (node: n.MemberExpression) { const { property } = node // something.(top|parent) - if (n.Identifier.check(property) && ['parent', 'top', 'location'].includes(property.name)) { + if (n.Identifier.check(property) && replaceableProps.includes(property.name)) { return property.name } // something['(top|parent)'] - if (n.Literal.check(property) && ['parent', 'top', 'location'].includes(String(property.value))) { + if (n.Literal.check(property) && replaceableProps.includes(String(property.value))) { return String(property.value) } @@ -104,6 +106,7 @@ export const jsRules: Visitor<{}> = { } path.replace(resolveWindowReference(path.get('object').node, prop)) + this.reportChanged() return false }, @@ -120,6 +123,7 @@ export const jsRules: Visitor<{}> = { if (isAssignee && node.name === 'location') { // `location = 'something'`, rewrite to intercepted href setter since relative urls can break this path.replace(b.memberExpression(resolveLocationReference(), b.identifier('href'))) + this.reportChanged() return false } @@ -147,40 +151,38 @@ export const jsRules: Visitor<{}> = { } } - if (path.scope.declares(node.name)) { - // identifier has been declared in local scope, don't care about replacing + // identifier has been declared in local scope, don't care about replacing + if (!replaceableProps.includes(node.name) || path.scope.declares(node.name)) { return this.traverse(path) } - if (node.name === 'location') { - path.replace(resolveLocationReference()) - - return false - } + switch (node.name) { + case 'location': + path.replace(resolveLocationReference()) + this.reportChanged() - if (['parent', 'top'].includes(node.name)) { - path.replace(resolveWindowReference(globalIdentifier, node.name)) + return false + case 'parent': + case 'top': + path.replace(resolveWindowReference(globalIdentifier, node.name)) + this.reportChanged() - return false + return false + default: + return this.traverse(path) } - - this.traverse(path) }, visitAssignmentExpression (path) { const { node } = path - const finish = () => { - this.traverse(path) - } - if (!n.MemberExpression.check(node.left)) { - return finish() + return this.traverse(path) } const propBeingSet = getReplaceablePropOfMemberExpression(node.left) if (!propBeingSet) { - return finish() + return this.traverse(path) } if (node.operator !== '=') { @@ -194,6 +196,7 @@ export const jsRules: Visitor<{}> = { const objBeingSetOn = node.left.object path.replace(resolveWindowReference(objBeingSetOn, propBeingSet, node.right)) + this.reportChanged() return false }, diff --git a/packages/rewriter/lib/js.ts b/packages/rewriter/lib/js.ts index 3e2cb7c09f08..f32eae4ad70d 100644 --- a/packages/rewriter/lib/js.ts +++ b/packages/rewriter/lib/js.ts @@ -32,7 +32,13 @@ export function rewriteJsSourceMap (url: string, js: string, inputSourceMap: any const ast = recast.parse(js, { sourceFileName }) - astTypes.visit(ast, jsRules) + const visitor = astTypes.PathVisitor.fromMethodsObject(jsRules) + + visitor.visit(ast) + + if (!visitor.wasChangeReported() && inputSourceMap) { + return inputSourceMap + } return recast.print(ast, { inputSourceMap, @@ -50,18 +56,32 @@ export function rewriteJsSourceMap (url: string, js: string, inputSourceMap: any export function _rewriteJsUnsafe (url: string, js: string, deferSourceMapRewrite?: DeferSourceMapRewriteFn): string { const ast = recast.parse(js) + let didRewrite: boolean + try { - astTypes.visit(ast, jsRules) + const visitor = astTypes.PathVisitor.fromMethodsObject(jsRules) + + visitor.visit(ast) + + didRewrite = visitor.wasChangeReported() } catch (err: any) { // if visiting fails, it points to a bug in our rewriting logic, so raise the error to the driver return _generateDriverError(url, err) } - const { code } = recast.print(ast, defaultPrintOpts) + let rewritten: string + + if (didRewrite) { + const { code } = recast.print(ast, defaultPrintOpts) + + rewritten = code + } else { + rewritten = js + } if (!deferSourceMapRewrite) { // no sourcemaps - return sourceMaps.stripMappingUrl(code) + return sourceMaps.stripMappingUrl(rewritten) } // get an ID that can be used to lazy-generate the source map later @@ -71,7 +91,7 @@ export function _rewriteJsUnsafe (url: string, js: string, deferSourceMapRewrite // using a relative URL ensures that required cookies + other headers are sent along // and can be reused if the user's sourcemap requires an HTTP request to be made `/__cypress/source-maps/${sourceMapId}.map`, - code, + rewritten, ) } diff --git a/packages/rewriter/test/unit/js-spec.ts b/packages/rewriter/test/unit/js-spec.ts index 479642d936d7..640fb657cfb4 100644 --- a/packages/rewriter/test/unit/js-spec.ts +++ b/packages/rewriter/test/unit/js-spec.ts @@ -164,7 +164,7 @@ describe('js rewriter', function () { err.stack = 'stack' - sinon.stub(astTypes, 'visit').throws(err) + sinon.stub(astTypes.PathVisitor, 'fromMethodsObject').throws(err) const actual = _rewriteJsUnsafe(URL, 'console.log()')