Skip to content

Commit

Permalink
Reduce dependency on lodash functions: values, extends (#11798)
Browse files Browse the repository at this point in the history
* Replace lodash 'values' usage with Object.keys => .map(obj[key])

* Block scoping: refactor letReferences, outsideLetReferences as objects of type Map

* Remove lodash dependency from babel-plugin-transform-block-scoping

* Fixup: Add missing Object.keys call

* Fixup: Update remaining property accessors

* Coerce Map.values() iterator results into an array via spread operator

* Fixup: Map.put -> Map.set

* Fixup: undo incorrect variable de-duplication

* Replace array-spread-plus-map combination with Array.from call

* Extract an extendMap function as an attempt to create an optimization boundary

* Experiment: cast objects to string (eliminates one Map/object difference per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map )

* Fixup: perform String cast on map keys, not values

* Revert "Fixup: perform String cast on map keys, not values"

This reverts commit abdd147.

* Revert "Experiment: cast objects to string (eliminates one Map/object difference per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map )"

This reverts commit a4035c8.

* Experiment: filter keys via Object.prototype.hasOwnProperty.call

* Revert "Experiment: filter keys via Object.prototype.hasOwnProperty.call"

This reverts commit 491c093.

* Migrate back from Map-based reference storage to Object-based storage; access performance appears much improved for Object property access

* Revert "Migrate back from Map-based reference storage to Object-based storage; access performance appears much improved for Object property access"

This reverts commit 2119acc.

* Iterate over a clone of outsideRefs keys

* Revert "Extract an extendMap function as an attempt to create an optimization boundary"

This reverts commit 85689f2.

* Fixup: migrate remaining Object property access to Map.get in tdz module
  • Loading branch information
jayaddison committed Jul 8, 2020
1 parent 58cfaf2 commit bff6298
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 24 deletions.
3 changes: 1 addition & 2 deletions packages/babel-plugin-transform-block-scoping/package.json
Expand Up @@ -13,8 +13,7 @@
},
"main": "lib/index.js",
"dependencies": {
"@babel/helper-plugin-utils": "^7.10.4",
"lodash": "^4.17.13"
"@babel/helper-plugin-utils": "^7.10.4"
},
"keywords": [
"babel-plugin"
Expand Down
46 changes: 25 additions & 21 deletions packages/babel-plugin-transform-block-scoping/src/index.js
Expand Up @@ -2,8 +2,6 @@ import { declare } from "@babel/helper-plugin-utils";
import type NodePath from "@babel/traverse";
import type Scope from "@babel/traverse";
import { visitor as tdzVisitor } from "./tdz";
import values from "lodash/values";
import extend from "lodash/extend";
import { traverse, template, types as t } from "@babel/core";

const DONE = new WeakSet();
Expand Down Expand Up @@ -195,7 +193,7 @@ const letReferenceBlockVisitor = traverse.visitors.merge([
const letReferenceFunctionVisitor = traverse.visitors.merge([
{
ReferencedIdentifier(path, state) {
const ref = state.letReferences[path.node.name];
const ref = state.letReferences.get(path.node.name);

// not a part of our scope
if (!ref) return;
Expand Down Expand Up @@ -250,7 +248,7 @@ const continuationVisitor = {
if (path.isAssignmentExpression() || path.isUpdateExpression()) {
for (const name of Object.keys(path.getBindingIdentifiers())) {
if (
state.outsideReferences[name] !==
state.outsideReferences.get(name) !==
path.scope.getBindingIdentifier(name)
) {
continue;
Expand Down Expand Up @@ -359,9 +357,9 @@ class BlockScoping {
this.blockPath = blockPath;
this.block = blockPath.node;

this.outsideLetReferences = Object.create(null);
this.outsideLetReferences = new Map();
this.hasLetReferences = false;
this.letReferences = Object.create(null);
this.letReferences = new Map();
this.body = [];

if (loopPath) {
Expand Down Expand Up @@ -447,8 +445,8 @@ class BlockScoping {
blockScope.getFunctionParent() || blockScope.getProgramParent();
const letRefs = this.letReferences;

for (const key of Object.keys(letRefs)) {
const ref = letRefs[key];
for (const key of letRefs.keys()) {
const ref = letRefs.get(key);
const binding = blockScope.getBinding(ref.name);
if (!binding) continue;
if (binding.kind === "let" || binding.kind === "const") {
Expand Down Expand Up @@ -476,10 +474,10 @@ class BlockScoping {
// those in upper scopes and then if they do, generate a uid
// for them and replace all references with it

for (const key of Object.keys(letRefs)) {
for (const key of letRefs.keys()) {
// just an Identifier node we collected in `getLetReferences`
// this is the defining identifier of a declaration
const ref = letRefs[key];
const ref = letRefs.get(key);

// todo: could skip this if the colliding binding is in another function
if (scope.parentHasBinding(key) || scope.hasGlobal(key)) {
Expand All @@ -496,8 +494,8 @@ class BlockScoping {
}
}

for (const key of Object.keys(outsideLetRefs)) {
const ref = letRefs[key];
for (const key of outsideLetRefs.keys()) {
const ref = letRefs.get(key);
// check for collisions with a for loop's init variable and the enclosing scope's bindings
// https://github.com/babel/babel/issues/8498
if (isInLoop(this.blockPath) && blockPathScope.hasOwnBinding(key)) {
Expand All @@ -519,20 +517,21 @@ class BlockScoping {

// remap loop heads with colliding variables
if (this.loop) {
for (const name of Object.keys(outsideRefs)) {
const id = outsideRefs[name];
// nb: clone outsideRefs keys since the map is modified within the loop
for (const name of [...outsideRefs.keys()]) {
const id = outsideRefs.get(name);

if (
this.scope.hasGlobal(id.name) ||
this.scope.parentHasBinding(id.name)
) {
delete outsideRefs[id.name];
delete this.letReferences[id.name];
outsideRefs.delete(id.name);
this.letReferences.delete(id.name);

this.scope.rename(id.name);

this.letReferences[id.name] = id;
outsideRefs[id.name] = id;
this.letReferences.set(id.name, id);
outsideRefs.set(id.name, id);
}
}
}
Expand All @@ -545,7 +544,7 @@ class BlockScoping {
this.hoistVarDeclarations();

// turn outsideLetReferences into an array
const args = values(outsideRefs).map(id => t.cloneNode(id));
const args = Array.from(outsideRefs.values(), node => t.cloneNode(node));
const params = args.map(id => t.cloneNode(id));

const isSwitch = this.blockPath.isSwitchStatement();
Expand Down Expand Up @@ -702,7 +701,10 @@ class BlockScoping {
const init = this.loop.left || this.loop.init;
if (isBlockScoped(init)) {
declarators.push(init);
extend(this.outsideLetReferences, t.getBindingIdentifiers(init));
const names = t.getBindingIdentifiers(init);
for (const name of Object.keys(names)) {
this.outsideLetReferences.set(name, names[name]);
}
}
}

Expand Down Expand Up @@ -751,7 +753,9 @@ class BlockScoping {
// declaration, rather than (for example) mistakenly including the
// parameters of a function declaration. Fixes #4880.
const keys = t.getBindingIdentifiers(declar, false, true);
extend(this.letReferences, keys);
for (const key of Object.keys(keys)) {
this.letReferences.set(key, keys[key]);
}
this.hasLetReferences = true;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/babel-plugin-transform-block-scoping/src/tdz.js
Expand Up @@ -20,7 +20,7 @@ function buildTDZAssert(node, state) {
}

function isReference(node, scope, state) {
const declared = state.letReferences[node.name];
const declared = state.letReferences.get(node.name);
if (!declared) return false;

// declared node is different in this scope
Expand Down

0 comments on commit bff6298

Please sign in to comment.