Skip to content

Commit 71c6bb5

Browse files
vzaidmanfacebook-github-bot
authored andcommitted
re-implement babel traverse cache pollution bug workaround using a second copy of babel traverse (#1340)
Summary: Pull Request resolved: #1340 ### The issue Traverse in babel 7 is currently polluting the cache with BabelNode that has no `hub` entry (see babel/babel#6437). Since the traverse cache is used by other babel operations (mostly transform) which might expect `hub` to be present, this breaks the code in all kind of unexpected scenarios. While this issue has been fixed in babel 8 (not released at this moment), it is still an issue in the latest version of babel 7. ### The previous workaround We've implemented a workaround for that issue in #854 (comment) however updating to the latest version of `babel/traverse`, that workaround is not working anymore (more about that below). ### The new workaround Instead, we are implementing a different workaround that installs traverse twice, including installing it's cache file twice. We use the second copy of traverse `babel/traverse--for-generate-function-map` only in `forEachMapping`, allowing the rest of the system to use the traverse caching from the main copy of traverse (`babel/traverse`) without the pollution issue mentioned above. ### Why the previous workaround stopped working Due to the use of a `let export` in the latest version of traverse cache, and how it's used in the latest version, we can't re-write `traverse.cache.path` anymore. This cache is exported with a `let export`: https://github.com/babel/babel/blob/5ebab544af2f1c6fc6abdaae6f4e5426975c9a16/packages/babel-traverse/src/cache.ts#L6-L20 And it compiles to: ``` let pathsCache = exports.path = new WeakMap(); function clearPath() { exports.path = pathsCache = new WeakMap(); } ``` and then used like this: ``` function getCachedPaths(hub, parent) { // ... pathsCache.get(hub); // ... } ``` Which means that re-writing the export like we used to do breaks the traverse cache because `exports.path` is re-written, but not `pathsCache` while the latter is used inside the file: ``` const previousCache = traverse.cache.path; traverse.cache.clearPath(); traverse(ast, { /* settings */ }); // this line is breaking the traverse cache traverse.cache.path = previousCache; ``` Reviewed By: robhogan Differential Revision: D61917782 fbshipit-source-id: 2ecc3133c1f9ecfe4912cb1fe2bf6151127e8e28
1 parent 386f97e commit 71c6bb5

File tree

3 files changed

+7
-8
lines changed

3 files changed

+7
-8
lines changed

packages/metro-source-map/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
},
1414
"dependencies": {
1515
"@babel/traverse": "^7.20.0",
16+
"@babel/traverse--for-generate-function-map": "npm:@babel/traverse@^7.20.0",
1617
"@babel/types": "^7.20.0",
1718
"flow-enums-runtime": "^0.0.6",
1819
"invariant": "^2.2.4",

packages/metro-source-map/src/generateFunctionMap.js

+5-7
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ import type {NodePath} from '@babel/traverse';
1717
import type {Node} from '@babel/types';
1818
import type {MetroBabelFileMetadata} from 'metro-babel-transformer';
1919

20-
import traverse from '@babel/traverse';
20+
// $FlowFixMe[cannot-resolve-module] - resolves to @babel/traverse
21+
import traverseForGenerateFunctionMap from '@babel/traverse--for-generate-function-map';
2122
import {
2223
isAssignmentExpression,
2324
isClassBody,
@@ -222,19 +223,16 @@ function forEachMapping(
222223

223224
// Traversing populates/pollutes the path cache (`traverse.cache.path`) with
224225
// values missing the `hub` property needed by Babel transformation, so we
225-
// save, clear, and restore the cache around our traversal.
226-
// See: https://github.com/facebook/metro/pull/854#issuecomment-1336499395
227-
const previousCache = traverse.cache.path;
228-
traverse.cache.clearPath();
229-
traverse(ast, {
226+
// use a separate copy of traverse to populate a separate cache to not pollute
227+
// the main @babel/traverse cache. See: https://github.com/facebook/metro/pull/1340
228+
traverseForGenerateFunctionMap(ast, {
230229
// Our visitor doesn't care about scope
231230
noScope: true,
232231

233232
Function: visitor,
234233
Program: visitor,
235234
Class: visitor,
236235
});
237-
traverse.cache.path = previousCache;
238236
}
239237

240238
const ANONYMOUS_NAME = '<anonymous>';

yarn.lock

+1-1
Original file line numberDiff line numberDiff line change
@@ -1089,7 +1089,7 @@
10891089
"@babel/parser" "^7.22.5"
10901090
"@babel/types" "^7.22.5"
10911091

1092-
"@babel/traverse@^7.19.1", "@babel/traverse@^7.20.0", "@babel/traverse@^7.20.1", "@babel/traverse@^7.20.5", "@babel/traverse@^7.4.3":
1092+
"@babel/traverse--for-generate-function-map@npm:@babel/traverse@^7.20.0", "@babel/traverse@^7.19.1", "@babel/traverse@^7.20.0", "@babel/traverse@^7.20.1", "@babel/traverse@^7.20.5", "@babel/traverse@^7.4.3":
10931093
version "7.20.5"
10941094
resolved "https://registry.yarnpkg.com/@babel/traverse/-/traverse-7.20.5.tgz#78eb244bea8270fdda1ef9af22a5d5e5b7e57133"
10951095
integrity sha512-WM5ZNN3JITQIq9tFZaw1ojLU3WgWdtkxnhM1AegMS+PvHjkM5IXjmYEGY7yukz5XS4sJyEf2VzWjI8uAavhxBQ==

0 commit comments

Comments
 (0)