diff --git a/src/babel/transformation/transformers/index.js b/src/babel/transformation/transformers/index.js index b44ba74e3a..54bba5495f 100644 --- a/src/babel/transformation/transformers/index.js +++ b/src/babel/transformation/transformers/index.js @@ -25,8 +25,6 @@ export default { "spec.blockScopedFunctions": require("./spec/block-scoped-functions"), "optimisation.react.constantElements": require("./optimisation/react.constant-elements"), "optimisation.react.inlineElements": require("./optimisation/react.inline-elements"), - reactCompat: require("./other/react-compat"), - react: require("./other/react"), "es7.comprehensions": require("./es7/comprehensions"), "es6.classes": require("./es6/classes"), asyncToGenerator: require("./other/async-to-generator"), @@ -60,6 +58,8 @@ export default { "es6.destructuring": require("./es6/destructuring"), "es6.blockScoping": require("./es6/block-scoping"), "es6.spec.blockScoping": require("./es6/spec.block-scoping"), + reactCompat: require("./other/react-compat"), + react: require("./other/react"), // es6 syntax transformation is **forbidden** past this point since regenerator will chuck a massive // hissy fit diff --git a/src/babel/transformation/transformers/optimisation/react.constant-elements.js b/src/babel/transformation/transformers/optimisation/react.constant-elements.js index 42cd5b8be0..be28455c82 100644 --- a/src/babel/transformation/transformers/optimisation/react.constant-elements.js +++ b/src/babel/transformation/transformers/optimisation/react.constant-elements.js @@ -35,9 +35,8 @@ export function JSXElement(node, parent, scope, file) { this.traverse(immutabilityVisitor, state); if (state.isImmutable) { - this.hoist(); - this.skip(); + return this.hoist(); + } else { + node._hoisted = true; } - - node._hoisted = true; } diff --git a/src/babel/transformation/transformers/other/react-compat.js b/src/babel/transformation/transformers/other/react-compat.js index 0546d203ef..9119ec9860 100644 --- a/src/babel/transformation/transformers/other/react-compat.js +++ b/src/babel/transformation/transformers/other/react-compat.js @@ -6,7 +6,8 @@ export function manipulateOptions(opts) { } export var metadata = { - optional: true + optional: true, + group: "builtin-advanced" }; require("../../helpers/build-react-transformer")(exports, { diff --git a/src/babel/transformation/transformers/other/react.js b/src/babel/transformation/transformers/other/react.js index a729671dd3..1cceee2fae 100644 --- a/src/babel/transformation/transformers/other/react.js +++ b/src/babel/transformation/transformers/other/react.js @@ -3,6 +3,10 @@ import * as t from "../../../types"; var JSX_ANNOTATION_REGEX = /^\*\s*@jsx\s+([^\s]+)/; +export var metadata = { + group: "builtin-advanced" +}; + export function Program(node, parent, scope, file) { var id = file.opts.jsxPragma; diff --git a/src/babel/traversal/path/hoister.js b/src/babel/traversal/path/hoister.js index 137fa6c233..4ba7fb47e3 100644 --- a/src/babel/traversal/path/hoister.js +++ b/src/babel/traversal/path/hoister.js @@ -2,28 +2,24 @@ import * as react from "../../transformation/helpers/react"; import * as t from "../../types"; var referenceVisitor = { - enter(node, parent, scope, state) { + ReferencedIdentifier(node, parent, scope, state) { if (this.isJSXIdentifier() && react.isCompatTag(node.name)) { return; } - if (this.isJSXIdentifier() || this.isIdentifier()) { - // direct references that we need to track to hoist this to the highest scope we can - if (this.isReferenced()) { - var bindingInfo = scope.getBinding(node.name); + // direct references that we need to track to hoist this to the highest scope we can + var bindingInfo = scope.getBinding(node.name); + if (!bindingInfo) return; - // this binding isn't accessible from the parent scope so we can safely ignore it - // eg. it's in a closure etc - if (bindingInfo !== state.scope.getBinding(node.name)) return; + // this binding isn't accessible from the parent scope so we can safely ignore it + // eg. it's in a closure etc + if (bindingInfo !== state.scope.getBinding(node.name)) return; - if (bindingInfo) { - if (bindingInfo.constant) { - state.bindings[node.name] = bindingInfo; - } else { - state.foundIncompatible = true; - this.stop(); - } - } + if (bindingInfo.constant) { + state.bindings[node.name] = bindingInfo; + } else { + for (var violationPath of (bindingInfo.constantViolations: Array)) { + state.breakOnScopePaths.push(violationPath.scope.path); } } } @@ -31,10 +27,10 @@ var referenceVisitor = { export default class PathHoister { constructor(path, scope) { - this.foundIncompatible = false; + this.breakOnScopePaths = []; this.bindings = {}; - this.scope = scope; this.scopes = []; + this.scope = scope; this.path = path; } @@ -45,32 +41,41 @@ export default class PathHoister { return false; } } + return true; } getCompatibleScopes() { - var checkScope = this.path.scope; + var scope = this.path.scope; do { - if (this.isCompatibleScope(checkScope)) { - this.scopes.push(checkScope); + if (this.isCompatibleScope(scope)) { + this.scopes.push(scope); } else { break; } - } while(checkScope = checkScope.parent); + + if (this.breakOnScopePaths.indexOf(scope.path) >= 0) { + break; + } + } while(scope = scope.parent); } getAttachmentPath() { var scopes = this.scopes; var scope = scopes.pop(); + if (!scope) return; if (scope.path.isFunction()) { - if (this.hasNonParamBindings()) { - // can't be attached to this scope - return this.getNextScopeStatementParent(); - } else { + if (this.hasOwnParamBindings(scope)) { + // should ignore this scope since it's ourselves + if (this.scope.is(scope)) return; + // needs to be attached to the body return scope.path.get("body").get("body")[0]; + } else { + // doesn't need to be be attached to this scope + return this.getNextScopeStatementParent(); } } else if (scope.path.isProgram()) { return this.getNextScopeStatementParent(); @@ -82,10 +87,12 @@ export default class PathHoister { if (scope) return scope.path.getStatementParent(); } - hasNonParamBindings() { + hasOwnParamBindings(scope) { for (var name in this.bindings) { + if (!scope.hasOwnBinding(name)) continue + var binding = this.bindings[name]; - if (binding.kind !== "param") return true; + if (binding.kind === "param") return true; } return false; } @@ -96,7 +103,6 @@ export default class PathHoister { node._hoisted = true; this.path.traverse(referenceVisitor, this); - if (this.foundIncompatible) return; this.getCompatibleScopes(); @@ -119,6 +125,6 @@ export default class PathHoister { uid = t.jSXExpressionContainer(uid); } - this.path.replaceWith(uid); + return uid; } } diff --git a/src/babel/traversal/scope/binding.js b/src/babel/traversal/scope/binding.js index 9e8b9eb181..88dfcdcbda 100644 --- a/src/babel/traversal/scope/binding.js +++ b/src/babel/traversal/scope/binding.js @@ -2,10 +2,13 @@ import * as t from "../../types"; export default class Binding { constructor({ identifier, scope, path, kind }) { + this.constantViolations = []; + this.constant = true; + this.identifier = identifier; this.references = 0; this.referenced = false; - this.constant = true; + this.scope = scope; this.path = path; this.kind = kind; @@ -51,8 +54,9 @@ export default class Binding { * Description */ - reassign() { + reassign(path) { this.constant = false; + this.constantViolations.push(path); if (this.typeAnnotationInferred) { // destroy the inferred typeAnnotation diff --git a/src/babel/traversal/scope/index.js b/src/babel/traversal/scope/index.js index 2ebde17cf1..6e586b6ec1 100644 --- a/src/babel/traversal/scope/index.js +++ b/src/babel/traversal/scope/index.js @@ -163,6 +163,17 @@ export default class Scope { traverse(node, opts, this, state, this.path); } + + /** + * Since `Scope` instances are unique to their traversal we need some other + * way to compare if scopes are the same. Here we just compare `this.bindings` + * as it will be the same across all instances. + */ + + is(scope) { + return this.bindings === scope.bindings; + } + /** * Description */ @@ -435,11 +446,13 @@ export default class Scope { for (var name in ids) { var binding = this.getBinding(name); if (!binding) continue; + if (right) { var rightType = right.typeAnnotation; if (rightType && binding.isCompatibleWithType(rightType)) continue; } - binding.reassign(); + + binding.reassign(left, right); } } diff --git a/test/core/fixtures/transformation/optimisation.react.constant-elements/global-reference/actual.js b/test/core/fixtures/transformation/optimisation.react.constant-elements/global-reference/actual.js new file mode 100644 index 0000000000..dbaf45c6d7 --- /dev/null +++ b/test/core/fixtures/transformation/optimisation.react.constant-elements/global-reference/actual.js @@ -0,0 +1,5 @@ +var Foo = React.createClass({ + render: function render() { + return
; + } +}); diff --git a/test/core/fixtures/transformation/optimisation.react.constant-elements/global-reference/expected.js b/test/core/fixtures/transformation/optimisation.react.constant-elements/global-reference/expected.js new file mode 100644 index 0000000000..bba75b60d6 --- /dev/null +++ b/test/core/fixtures/transformation/optimisation.react.constant-elements/global-reference/expected.js @@ -0,0 +1,9 @@ +"use strict"; + +var _ref = ; + +var Foo = React.createClass({ + render: function render() { + return _ref; + } +}); diff --git a/test/core/fixtures/transformation/optimisation.react.constant-elements/parameter-reference/actual.js b/test/core/fixtures/transformation/optimisation.react.constant-elements/parameter-reference/actual.js new file mode 100644 index 0000000000..0b1379990f --- /dev/null +++ b/test/core/fixtures/transformation/optimisation.react.constant-elements/parameter-reference/actual.js @@ -0,0 +1,11 @@ +function render(text) { + return function () { + return
+
+