From 5df70e6a94e0cd302a4a7e1d7b8d85b769757e73 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Sat, 2 Sep 2017 01:03:10 -0400 Subject: [PATCH] Fix bad Scope#parent caching (#6155) * Fix bad Scope#parent caching Now, we traverse the path until we find a parent scope. Fixes #6057. * Fix bad merge * Remove cached data * I need to stop using Github editor * Fix infinite loops due to scopable paths being moved up --- .../regression/6057-expanded/actual.js | 9 +++ .../regression/6057-expanded/expected.js | 56 +++++++++++++++++++ .../regression/6057-expanded/options.json | 4 ++ .../fixtures/regression/6057-simple/actual.js | 5 ++ .../regression/6057-simple/expected.js | 9 +++ .../regression/6057-simple/options.json | 3 + packages/babel-traverse/src/path/index.js | 9 +-- packages/babel-traverse/src/scope/index.js | 51 ++++++++--------- 8 files changed, 109 insertions(+), 37 deletions(-) create mode 100644 packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/actual.js create mode 100644 packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/expected.js create mode 100644 packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/options.json create mode 100644 packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/actual.js create mode 100644 packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/expected.js create mode 100644 packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/options.json diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/actual.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/actual.js new file mode 100644 index 0000000000..51a73a9bde --- /dev/null +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/actual.js @@ -0,0 +1,9 @@ +import args from 'utils/url/args'; + +export default class App extends Component { + exportType = '' + + componentDidMount() { + this.exportType = args.get('type', window.location.href); + } +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/expected.js new file mode 100644 index 0000000000..6185b7b9fa --- /dev/null +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/expected.js @@ -0,0 +1,56 @@ +"use strict"; + +Object.defineProperty(exports, "__esModule", { + value: true +}); +exports.default = void 0; + +var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) { return typeof obj; } : function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; }; + +var _createClass = function () { function defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } } return function (Constructor, protoProps, staticProps) { if (protoProps) defineProperties(Constructor.prototype, protoProps); if (staticProps) defineProperties(Constructor, staticProps); return Constructor; }; }(); + +var _args = require("utils/url/args"); + +var _args2 = _interopRequireDefault(_args); + +function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } + +function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } } + +function _possibleConstructorReturn(self, call) { if (call && (_typeof(call) === "object" || typeof call === "function")) { return call; } if (!self) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return self; } + +function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function"); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; } + +var App = function (_Component) { + _inherits(App, _Component); + + function App() { + var _ref; + + var _temp, _this; + + _classCallCheck(this, App); + + for (var _len = arguments.length, args = Array(_len), _key = 0; _key < _len; _key++) { + args[_key] = arguments[_key]; + } + + return _possibleConstructorReturn(_this, (_temp = _this = _possibleConstructorReturn(this, (_ref = App.__proto__ || Object.getPrototypeOf(App)).call.apply(_ref, [this].concat(args))), Object.defineProperty(_this, "exportType", { + configurable: true, + enumerable: true, + writable: true, + value: '' + }), _temp)); + } + + _createClass(App, [{ + key: "componentDidMount", + value: function componentDidMount() { + this.exportType = _args2.default.get('type', window.location.href); + } + }]); + + return App; +}(Component); + +exports.default = App; diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/options.json b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/options.json new file mode 100644 index 0000000000..b30d23db4d --- /dev/null +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-expanded/options.json @@ -0,0 +1,4 @@ +{ + "presets": ["es2015"], + "plugins": ["transform-class-properties"] +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/actual.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/actual.js new file mode 100644 index 0000000000..a59c906934 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/actual.js @@ -0,0 +1,5 @@ +const a = 'bar'; +function foo(...a) { + return a; +} + diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/expected.js new file mode 100644 index 0000000000..d04c1fea58 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/expected.js @@ -0,0 +1,9 @@ +const a = 'bar'; + +function foo() { + for (var _len = arguments.length, a = Array(_len), _key = 0; _key < _len; _key++) { + a[_key] = arguments[_key]; + } + + return a; +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/options.json b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/options.json new file mode 100644 index 0000000000..82bc9b4032 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/regression/6057-simple/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["transform-es2015-parameters"] +} diff --git a/packages/babel-traverse/src/path/index.js b/packages/babel-traverse/src/path/index.js index dbbd8e2bab..43725fc344 100644 --- a/packages/babel-traverse/src/path/index.js +++ b/packages/babel-traverse/src/path/index.js @@ -105,14 +105,7 @@ export default class NodePath { } getScope(scope: Scope) { - let ourScope = scope; - - // we're entering a new scope so let's construct it! - if (this.isScope()) { - ourScope = new Scope(this, scope); - } - - return ourScope; + return this.isScope() ? new Scope(this) : scope; } setData(key: string, val: any): any { diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js index 8561027b6d..40d62a84fb 100644 --- a/packages/babel-traverse/src/scope/index.js +++ b/packages/babel-traverse/src/scope/index.js @@ -16,25 +16,6 @@ import { scope as scopeCache } from "../cache"; // See `warnOnFlowBinding`. let _crawlCallsCount = 0; -/** - * To avoid creating a new Scope instance for each traversal, we maintain a cache on the - * node itself containing all scopes it has been associated with. - */ - -function getCache(path, parentScope, self) { - const scopes: Array = scopeCache.get(path.node) || []; - - for (const scope of scopes) { - if (scope.parent === parentScope && scope.path === path) return scope; - } - - scopes.push(self); - - if (!scopeCache.has(path.node)) { - scopeCache.set(path.node, scopes); - } -} - // Recursively gathers the identifying names of a node. function gatherNodeParts(node: Object, parts: Array) { if (t.isModuleDeclaration(node)) { @@ -181,20 +162,19 @@ export default class Scope { * within. */ - constructor(path: NodePath, parentScope?: Scope) { - if (parentScope && parentScope.block === path.node) { - return parentScope; + constructor(path: NodePath) { + const { node } = path; + const cached = scopeCache.get(node); + // Sometimes, a scopable path is placed higher in the AST tree. + // In these cases, have to create a new Scope. + if (cached && cached.path === path) { + return cached; } - - const cached = getCache(path, parentScope, this); - if (cached) return cached; + scopeCache.set(node, this); this.uid = uid++; - this.parent = parentScope; - this.hub = path.hub; - this.parentBlock = path.parent; - this.block = path.node; + this.block = node; this.path = path; this.labels = new Map(); @@ -212,6 +192,19 @@ export default class Scope { static contextVariables = ["arguments", "undefined", "Infinity", "NaN"]; + get parent() { + const parent = this.path.findParent(p => p.isScope()); + return parent && parent.scope; + } + + get parentBlock() { + return this.path.parent; + } + + get hub() { + return this.path.hub; + } + /** * Traverse node with current scope and path. */