Skip to content

Commit

Permalink
Move NodePath cache out of the AST
Browse files Browse the repository at this point in the history
As mentioned on the task https://phabricator.babeljs.io/T7179 having
this cache on the AST leads to all sorts of portability and reuse
bugs.

This moves the cache into a clearable WeakMap which will fix the
following:

1. Moving the AST between different babel versions or tools will not
lead into sharing potentially outdated cached information

2. `.clear()` can be called on the cache by a plugin to clear
potentially outdated information. This is helpful when implementing two
seperate pipelines that should not share information.

I think the next step (which is harder, I tried) is to isolate cache and
make it live on a transform or pipeline level state (like the `hub`).

The reason it is hard is because the `babel-traverse` main API -- although
requires the state object to be passed -- not many callers do. To fix
this we should release a patch version that warns about this and fix all
the internal callers. Next couple of releases we can start throwing when
no state is passed (or we can create our own state).
  • Loading branch information
amasad committed Mar 7, 2016
1 parent 3667527 commit b9a893a
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/babel-traverse/src/index.js
Expand Up @@ -9,6 +9,7 @@ import * as t from "babel-types";
export { default as NodePath } from "./path";
export { default as Scope } from "./scope";
export { default as Hub } from "./hub";
export { default as cache } from "./path/cache";

This comment has been minimized.

Copy link
@jedwards1211

jedwards1211 Dec 29, 2020

Contributor

@amasad I'm not sure having this cache be global to a particular version of @babel/traverse solves this issue either, I'm still running into issues of information from one traversal unexpectedly leaking to another and clobbering visitors in #12570.

export { visitors };

export default function traverse(
Expand Down
25 changes: 25 additions & 0 deletions packages/babel-traverse/src/path/cache.js
@@ -0,0 +1,25 @@
let wm = new WeakMap();

// To implement clear we need to export a facade.
export default {
clear() {
wm = new WeakMap();
},

delete(k) {
return wm.delete(k)
},

get(k) {
return wm.get(k)
},

has(k) {
return wm.has(k)
},

set(k, v) {
wm.set(k, v);
return wm;
},
};
1 change: 0 additions & 1 deletion packages/babel-traverse/src/path/constants.js

This file was deleted.

8 changes: 6 additions & 2 deletions packages/babel-traverse/src/path/index.js
Expand Up @@ -4,12 +4,12 @@ import type Hub from "../hub";
import type TraversalContext from "../context";
import * as virtualTypes from "./lib/virtual-types";
import buildDebug from "debug";
import { PATH_CACHE_KEY } from "./constants";
import invariant from "invariant";
import traverse from "../index";
import assign from "lodash/object/assign";
import Scope from "../scope";
import * as t from "babel-types";
import cache from "./cache";

let debug = buildDebug("babel");

Expand Down Expand Up @@ -69,7 +69,11 @@ export default class NodePath {

let targetNode = container[key];

let paths = parent[PATH_CACHE_KEY] = parent[PATH_CACHE_KEY] || [];
let paths = cache.get(parent) || [];
if (!cache.has(parent)) {
cache.set(parent, paths);
}

let path;

for (let i = 0; i < paths.length; i++) {
Expand Down
4 changes: 2 additions & 2 deletions packages/babel-traverse/src/path/modification.js
@@ -1,7 +1,7 @@
/* eslint max-len: 0 */
// This file contains methods that modify the path/node in some ways.

import { PATH_CACHE_KEY } from "./constants";
import cache from "./cache";
import PathHoister from "./lib/hoister";
import NodePath from "./index";
import * as t from "babel-types";
Expand Down Expand Up @@ -136,7 +136,7 @@ export function insertAfter(nodes) {
export function updateSiblingKeys(fromIndex, incrementBy) {
if (!this.parent) return;

let paths = this.parent[PATH_CACHE_KEY];
let paths = cache.get(this.parent);
for (let i = 0; i < paths.length; i++) {
let path = paths[i];
if (path.key >= fromIndex) {
Expand Down

0 comments on commit b9a893a

Please sign in to comment.