From 342f9d5eb5aef7daea5ee9839d933dc67cf7cfe8 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Thu, 10 Mar 2016 11:47:33 -0800 Subject: [PATCH 1/5] Don't consider flow types as bindings --- .../test/fixtures/general/flow-declar/actual.js | 2 ++ .../test/fixtures/general/flow-declar/expected.js | 1 + .../test/fixtures/general/flow-declar/options.json | 3 +++ packages/babel-traverse/src/scope/index.js | 3 +++ 4 files changed, 9 insertions(+) create mode 100644 packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/actual.js create mode 100644 packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/expected.js create mode 100644 packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/options.json diff --git a/packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/actual.js b/packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/actual.js new file mode 100644 index 0000000000..e9ca299ae8 --- /dev/null +++ b/packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/actual.js @@ -0,0 +1,2 @@ +declare class foo {} +const foo = 1; diff --git a/packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/expected.js b/packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/expected.js new file mode 100644 index 0000000000..9375fc4abd --- /dev/null +++ b/packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/expected.js @@ -0,0 +1 @@ +var foo = 1; diff --git a/packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/options.json b/packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/options.json new file mode 100644 index 0000000000..954431d3a9 --- /dev/null +++ b/packages/babel-plugin-check-es2015-constants/test/fixtures/general/flow-declar/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["check-es2015-constants", "transform-es2015-block-scoping", "transform-flow-strip-types"] +} diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js index 084100c815..e05ad675ca 100644 --- a/packages/babel-traverse/src/scope/index.js +++ b/packages/babel-traverse/src/scope/index.js @@ -48,6 +48,9 @@ let collectorVisitor = { // this will be hit again once we traverse into it after this iteration if (path.isExportDeclaration() && path.get("declaration").isDeclaration()) return; + // Skip flow declarations + if (path.isFlow()) return; + // we've ran into a declaration! path.scope.getFunctionParent().registerDeclaration(path); }, From fd7b1c33860239e5914c76ede806c08c53639e88 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Thu, 10 Mar 2016 12:13:05 -0800 Subject: [PATCH 2/5] don't rely on scope to get the type alias --- packages/babel-core/test/api.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/babel-core/test/api.js b/packages/babel-core/test/api.js index 9422b018d9..f00636ebd2 100644 --- a/packages/babel-core/test/api.js +++ b/packages/babel-core/test/api.js @@ -89,14 +89,8 @@ suite("api", function () { new Plugin({ visitor: { Function: function(path) { - var node = path.node; - var scope = path.scope; - - var alias = scope - .getProgramParent() - .getBinding(node.returnType.typeAnnotation.id.name) - .path - .node; + var alias = path.scope.getProgramParent().path.get('body')[0].node; + if (!babel.types.isTypeAlias(alias)) return; // In case of `passPerPreset` being `false`, the // alias node is already removed by Flow plugin. From 7f4b57a7a4c14c2f563469ee142ddb9fc6a434ec Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Thu, 10 Mar 2016 12:35:15 -0800 Subject: [PATCH 3/5] Add warning instead of removing support --- packages/babel-traverse/src/scope/index.js | 23 ++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js index e05ad675ca..890d8f1c94 100644 --- a/packages/babel-traverse/src/scope/index.js +++ b/packages/babel-traverse/src/scope/index.js @@ -48,8 +48,8 @@ let collectorVisitor = { // this will be hit again once we traverse into it after this iteration if (path.isExportDeclaration() && path.get("declaration").isDeclaration()) return; - // Skip flow declarations - if (path.isFlow()) return; + // TODO(amasad): remove support for flow as bindings (See warning below). + //if (path.isFlow()) return; // we've ran into a declaration! path.scope.getFunctionParent().registerDeclaration(path); @@ -487,6 +487,11 @@ export default class Scope { this.checkBlockScopedCollisions(local, kind, name, id); } + // It's erroneous that we currently consider flow a binding, however, we can't + // remove it because people might be depending on it. See warning section + // in `getBinding` + if (local && local.path.isFlow()) local = null; + parent.references[name] = true; this.bindings[name] = new Binding({ @@ -837,17 +842,27 @@ export default class Scope { return this.getBindingIdentifier(name) === node; } + warnOnFlowBinding(binding) { + if (binding && binding.path.isFlow()) { + console.warn(` + You are using Flow declarations as bindings and it is not supported anymore + and will be removed in the next version 6.8. + `); + } + return binding; + } + getBinding(name: string) { let scope = this; do { let binding = scope.getOwnBinding(name); - if (binding) return binding; + if (binding) return this.warnOnFlowBinding(binding); } while (scope = scope.parent); } getOwnBinding(name: string) { - return this.bindings[name]; + return this.warnOnFlowBinding(this.bindings[name]); } getBindingIdentifier(name: string) { From 9f0dbf0234ed4e645901b15c4121fb9a41eae4f0 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Thu, 10 Mar 2016 12:54:02 -0800 Subject: [PATCH 4/5] Better warnings --- packages/babel-traverse/src/scope/index.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js index 890d8f1c94..a6033f72ce 100644 --- a/packages/babel-traverse/src/scope/index.js +++ b/packages/babel-traverse/src/scope/index.js @@ -682,6 +682,7 @@ export default class Scope { path.traverse(collectorVisitor, state); this.crawling = false; + this._warnOnFlowBinding = false; // register assignments for (let path of state.assignments) { // register undeclared bindings as globals @@ -712,6 +713,7 @@ export default class Scope { for (let path of state.constantViolations) { path.scope.registerConstantViolation(path); } + this._warnOnFlowBinding = true; } push(opts: { @@ -843,10 +845,11 @@ export default class Scope { } warnOnFlowBinding(binding) { - if (binding && binding.path.isFlow()) { + if (!this.crawling && this._warnOnFlowBinding && binding && binding.path.isFlow()) { console.warn(` - You are using Flow declarations as bindings and it is not supported anymore - and will be removed in the next version 6.8. + You or one of the Babel plugins you are using are using Flow declarations as bindings. + Support for this will be removed in version 6.8. To find out the caller, grep for this message + and change it to a \`console.trace()\`. `); } return binding; From 5a081a85720869a2cd33b2401a8522ac4c2d0be4 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Thu, 10 Mar 2016 13:00:45 -0800 Subject: [PATCH 5/5] Update comment, reformat message --- packages/babel-traverse/src/scope/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js index a6033f72ce..7d7abe4946 100644 --- a/packages/babel-traverse/src/scope/index.js +++ b/packages/babel-traverse/src/scope/index.js @@ -489,7 +489,7 @@ export default class Scope { // It's erroneous that we currently consider flow a binding, however, we can't // remove it because people might be depending on it. See warning section - // in `getBinding` + // in `warnOnFlowBinding`. if (local && local.path.isFlow()) local = null; parent.references[name] = true; @@ -848,8 +848,8 @@ export default class Scope { if (!this.crawling && this._warnOnFlowBinding && binding && binding.path.isFlow()) { console.warn(` You or one of the Babel plugins you are using are using Flow declarations as bindings. - Support for this will be removed in version 6.8. To find out the caller, grep for this message - and change it to a \`console.trace()\`. + Support for this will be removed in version 6.8. To find out the caller, grep for this + message and change it to a \`console.trace()\`. `); } return binding;