Revert "Use NodePath#hub as part of the paths cache key (#15725)"

This reverts commit abb5a7c.

packages/babel-core/src/transformation/index.ts

@@ -117,7 +117,7 @@ function* transformFile(file: File, pluginPasses: PluginPasses): Handler<void> {
       passes,
       file.opts.wrapPluginVisitorMethod,
     );
-    traverse(file.ast.program, visitor, file.scope, null, file.path, true);
+    traverse(file.ast, visitor, file.scope);
 
     for (const [plugin, pass] of passPairs) {
       const fn = plugin.post;

packages/babel-traverse/package.json

@@ -28,7 +28,6 @@
     "globals": "condition:BABEL_8_BREAKING ? ^13.5.0 : ^11.1.0"
   },
   "devDependencies": {
-    "@babel/core": "workspace:^",
     "@babel/helper-plugin-test-runner": "workspace:^"
   },
   "engines": {

packages/babel-traverse/src/cache.ts

@@ -1,13 +1,8 @@
 import type { Node } from "@babel/types";
 import type NodePath from "./path";
 import type Scope from "./scope";
-import type { HubInterface } from "./hub";
 
-let pathsCache: WeakMap<
-  HubInterface | typeof nullHub,
-  WeakMap<Node, Map<Node, NodePath>>
-> = new WeakMap();
-export { pathsCache as path };
+export let path: WeakMap<Node, Map<Node, NodePath>> = new WeakMap();
 export let scope: WeakMap<Node, Scope> = new WeakMap();
 
 export function clear() {
@@ -16,29 +11,9 @@ export function clear() {
 }
 
 export function clearPath() {
-  pathsCache = new WeakMap();
+  path = new WeakMap();
 }
 
 export function clearScope() {
   scope = new WeakMap();
 }
-
-// NodePath#hub can be null, but it's not a valid weakmap key because it
-// cannot be collected by GC. Use an object, knowing tht it will not be
-// collected anyway. It's not a memory leak because pathsCache.get(nullHub)
-// is itself a weakmap, so its entries can still be collected.
-const nullHub = Object.freeze({} as const);
-
-export function getCachedPaths(hub: HubInterface | null, parent: Node) {
-  return pathsCache.get(hub ?? nullHub)?.get(parent);
-}
-
-export function getOrCreateCachedPaths(hub: HubInterface | null, parent: Node) {
-  let parents = pathsCache.get(hub ?? nullHub);
-  if (!parents) pathsCache.set(hub ?? nullHub, (parents = new WeakMap()));
-
-  let paths = parents.get(parent);
-  if (!paths) parents.set(parent, (paths = new Map()));
-
-  return paths;
-}

packages/babel-traverse/src/index.ts

@@ -36,7 +36,6 @@ function traverse<S>(
   scope: Scope | undefined,
   state: S,
   parentPath?: NodePath,
-  visitSelf?: boolean,
 ): void;
 
 function traverse(
@@ -45,7 +44,6 @@ function traverse(
   scope?: Scope,
   state?: any,
   parentPath?: NodePath,
-  visitSelf?: boolean,
 ): void;
 
 function traverse<Options extends TraverseOptions>(
@@ -55,7 +53,6 @@ function traverse<Options extends TraverseOptions>(
   scope?: Scope,
   state?: any,
   parentPath?: NodePath,
-  visitSelf?: boolean,
 ) {
   if (!parent) return;
 
@@ -69,25 +66,13 @@ function traverse<Options extends TraverseOptions>(
     }
   }
 
-  if (!parentPath && visitSelf) {
-    throw new Error("visitSelf can only be used when providing a NodePath.");
-  }
-
   if (!VISITOR_KEYS[parent.type]) {
     return;
   }
 
   visitors.explode(opts as Visitor);
 
-  traverseNode(
-    parent,
-    opts as ExplodedVisitor,
-    scope,
-    state,
-    parentPath,
-    /* skipKeys */ null,
-    visitSelf,
-  );
+  traverseNode(parent, opts as ExplodedVisitor, scope, state, parentPath);
 }
 
 export default traverse;
@@ -115,6 +100,8 @@ traverse.node = function (
 
 traverse.clearNode = function (node: t.Node, opts?: RemovePropertiesOptions) {
   removeProperties(node, opts);
+
+  cache.path.delete(node);
 };
 
 traverse.removeProperties = function (

packages/babel-traverse/src/path/index.ts

@@ -8,7 +8,7 @@ import type { Visitor } from "../types";
 import Scope from "../scope";
 import { validate } from "@babel/types";
 import * as t from "@babel/types";
-import { getOrCreateCachedPaths } from "../cache";
+import { path as pathCache } from "../cache";
 import generator from "@babel/generator";
 
 // NodePath is split across many files.
@@ -92,7 +92,11 @@ class NodePath<T extends t.Node = t.Node> {
       // @ts-expect-error key must present in container
       container[key];
 
-    const paths = getOrCreateCachedPaths(hub, parent);
+    let paths = pathCache.get(parent);
+    if (!paths) {
+      paths = new Map();
+      pathCache.set(parent, paths);
+    }
 
     let path = paths.get(targetNode);
     if (!path) {

packages/babel-traverse/src/path/modification.ts

@@ -1,6 +1,6 @@
 // This file contains methods that modify the path/node in some ways.
 
-import { getCachedPaths } from "../cache";
+import { path as pathCache } from "../cache";
 import PathHoister from "./lib/hoister";
 import NodePath from "./index";
 import {
@@ -279,7 +279,7 @@ export function updateSiblingKeys(
 ) {
   if (!this.parent) return;
 
-  const paths = getCachedPaths(this.hub, this.parent) || ([] as never[]);
+  const paths = pathCache.get(this.parent);
 
   for (const [, path] of paths) {
     if (typeof path.key === "number" && path.key >= fromIndex) {

packages/babel-traverse/src/path/removal.ts

@@ -1,7 +1,7 @@
 // This file contains methods responsible for removing a node.
 
 import { hooks } from "./lib/removal-hooks";
-import { getCachedPaths } from "../cache";
+import { path as pathCache } from "../cache";
 import type NodePath from "./index";
 import { REMOVED, SHOULD_SKIP } from "./index";
 
@@ -46,9 +46,7 @@ export function _remove(this: NodePath) {
 export function _markRemoved(this: NodePath) {
   // this.shouldSkip = true; this.removed = true;
   this._traverseFlags |= SHOULD_SKIP | REMOVED;
-  if (this.parent) {
-    getCachedPaths(this.hub, this.parent).delete(this.node);
-  }
+  if (this.parent) pathCache.get(this.parent).delete(this.node);
   this.node = null;
 }
 

packages/babel-traverse/src/path/replacement.ts

@@ -3,7 +3,7 @@
 import { codeFrameColumns } from "@babel/code-frame";
 import traverse from "../index";
 import NodePath from "./index";
-import { getCachedPaths } from "../cache";
+import { path as pathCache } from "../cache";
 import { parse } from "@babel/parser";
 import {
   FUNCTION_TYPES,
@@ -47,7 +47,7 @@ export function replaceWithMultiple(
   nodes = this._verifyNodeList(nodes);
   inheritLeadingComments(nodes[0], this.node);
   inheritTrailingComments(nodes[nodes.length - 1], this.node);
-  getCachedPaths(this.hub, this.parent)?.delete(this.node);
+  pathCache.get(this.parent)?.delete(this.node);
   this.node =
     // @ts-expect-error this.key must present in this.container
     this.container[this.key] = null;
@@ -210,7 +210,7 @@ export function _replaceWith(this: NodePath, node: t.Node) {
   }
 
   this.debug(`Replace with ${node?.type}`);
-  getCachedPaths(this.hub, this.parent)?.set(node, this).delete(this.node);
+  pathCache.get(this.parent)?.set(node, this).delete(this.node);
 
   this.node =
     // @ts-expect-error this.key must present in this.container

packages/babel-traverse/src/traverse-node.ts

@@ -24,19 +24,13 @@ export function traverseNode<S = unknown>(
   state?: any,
   path?: NodePath,
   skipKeys?: Record<string, boolean>,
-  visitSelf?: boolean,
 ): boolean {
   const keys = VISITOR_KEYS[node.type];
   if (!keys) return false;
 
   const context = new TraversalContext(scope, opts, state, path);
-  if (visitSelf) {
-    if (skipKeys?.[path.parentKey]) return false;
-    return context.visitQueue([path]);
-  }
-
   for (const key of keys) {
-    if (skipKeys?.[key]) continue;
+    if (skipKeys && skipKeys[key]) continue;
     if (context.visit(node, key)) {
       return true;
     }

packages/babel-traverse/test/hub.js

@@ -1,46 +1,10 @@
-import { transformSync } from "@babel/core";
+import assert from "assert";
 import { Hub } from "../lib/index.js";
 
 describe("hub", function () {
   it("default buildError should return TypeError", function () {
     const hub = new Hub();
     const msg = "test_msg";
-    expect(hub.buildError(null, msg)).toEqual(new TypeError(msg));
-  });
-
-  it("should be preserved across nested traversals", function () {
-    let origHub;
-    let innerHub = {};
-    let exprHub;
-    function plugin({ types: t, traverse }) {
-      return {
-        visitor: {
-          Identifier(path) {
-            if (path.node.name !== "foo") return;
-            origHub = path.hub;
-
-            const mem = t.memberExpression(
-              t.identifier("property"),
-              t.identifier("access"),
-            );
-            traverse(mem, {
-              noScope: true,
-              Identifier(path) {
-                if (path.node.name === "property") innerHub = path.hub;
-              },
-            });
-            const [p2] = path.insertAfter(mem);
-
-            exprHub = p2.get("expression").hub;
-          },
-        },
-      };
-    }
-
-    transformSync("foo;", { configFile: false, plugins: [plugin] });
-
-    expect(origHub).toBeInstanceOf(Object);
-    expect(exprHub).toBe(origHub);
-    expect(innerHub).toBeUndefined();
+    assert.deepEqual(hub.buildError(null, msg), new TypeError(msg));
   });
 });

yarn.lock

@@ -3956,7 +3956,6 @@ __metadata:
   resolution: "@babel/traverse@workspace:packages/babel-traverse"
   dependencies:
     "@babel/code-frame": "workspace:^"
-    "@babel/core": "workspace:^"
     "@babel/generator": "workspace:^"
     "@babel/helper-environment-visitor": "workspace:^"
     "@babel/helper-function-name": "workspace:^"