Skip to content

Commit

Permalink
re-implement babel traverse cache pollution bug workaround using a se…
Browse files Browse the repository at this point in the history
…cond copy of babel traverse

Summary:
### The issue
Traverse in babel 7 is currently polluting the cache with nodes that have no `hub` entry (see babel/babel#6437). Since the traverse cache is used by other babel operations that expect `hub` to be present, this breaks the code in all kind of unexpected scenarios.
While this issue is fixed in babel 8 (not released at this moment), it's still an issue in the latest version of babel 7.

### The previous workaround
We implemented a workaround for it in #854 (comment) however in the latest version of `babel/traverse`, that workaround cannot be used 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 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;
```

Differential Revision: D61917782
  • Loading branch information
vzaidman authored and facebook-github-bot committed Aug 30, 2024
1 parent 5d63c99 commit bface4b
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 10 deletions.
1 change: 1 addition & 0 deletions packages/metro-source-map/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
},
"dependencies": {
"@babel/traverse": "^7.20.0",
"@babel/traverse--for-generate-function-map": "npm:@babel/traverse@^7.20.0",
"@babel/types": "^7.20.0",
"flow-enums-runtime": "^0.0.6",
"invariant": "^2.2.4",
Expand Down
13 changes: 4 additions & 9 deletions packages/metro-source-map/src/generateFunctionMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import type {NodePath} from '@babel/traverse';
import type {Node} from '@babel/types';
import type {MetroBabelFileMetadata} from 'metro-babel-transformer';

import traverse from '@babel/traverse';
// $FlowFixMe[cannot-resolve-module] - resolves to @babel/traverse
import traverseSecondInstallation from '@babel/traverse--for-generate-function-map';
import {
isAssignmentExpression,
isClassBody,
Expand Down Expand Up @@ -220,21 +221,15 @@ function forEachMapping(
) {
const visitor = getFunctionMapVisitor(context, pushMapping);

// Traversing populates/pollutes the path cache (`traverse.cache.path`) with
// values missing the `hub` property needed by Babel transformation, so we
// save, clear, and restore the cache around our traversal.
// See: https://github.com/facebook/metro/pull/854#issuecomment-1336499395
const previousCache = traverse.cache.path;
traverse.cache.clearPath();
traverse(ast, {
// TODO: improve comment
traverseSecondInstallation(ast, {
// Our visitor doesn't care about scope
noScope: true,

Function: visitor,
Program: visitor,
Class: visitor,
});
traverse.cache.path = previousCache;
}

const ANONYMOUS_NAME = '<anonymous>';
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,7 @@
"@babel/parser" "^7.22.5"
"@babel/types" "^7.22.5"

"@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":
"@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":
version "7.20.5"
resolved "https://registry.yarnpkg.com/@babel/traverse/-/traverse-7.20.5.tgz#78eb244bea8270fdda1ef9af22a5d5e5b7e57133"
integrity sha512-WM5ZNN3JITQIq9tFZaw1ojLU3WgWdtkxnhM1AegMS+PvHjkM5IXjmYEGY7yukz5XS4sJyEf2VzWjI8uAavhxBQ==
Expand Down

0 comments on commit bface4b

Please sign in to comment.