diff --git a/README.md b/README.md index 8d88bed..02dedd3 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,9 @@ Babel plugin to hoist nested functions to the outermost scope possible without changing their contract. -## Example +## Examples + +### Example 1 - basic hoisting **In** @@ -32,6 +34,36 @@ function renderApp () { } ``` +### Example 2 - nested method hoisting + +To enable this transformation, pass the `methods: true` option to the plugin (see below). +The output code depends on the ES2015 `Symbol` feature and the stage 2 class properties proposal. +You will _most likely_ want to run `babel-plugin-transform-class-properties` after `transform-hoist-nested-function`. + +**In** + +```js +class Foo { + bar () { + return () => this; + } +} +``` + +**Out** + +```js +const _hoistedMethod = new Symbol("_hoistedMethod"), + +class Foo { + [_hoistedMethod] = () => this; + + bar() { + return this[_hoistedMethod]; + } +} +``` + ## Motivation Patterns like [React "render callbacks"] @@ -69,7 +101,7 @@ factory() === factory(); // ⬅ value depends on whether foo() is hoisted ``` That last expression evaluates to `false` in plain JavaScript, but is `true` if `foo()` has been -hoisted. +hoisted. More fundamentally, **references to hoisted inner functions are allowed to escape their enclosing scopes**. You should determine whether this is appropriate for your code before using this plugin. @@ -96,10 +128,22 @@ $ npm install --save-dev babel-plugin-transform-hoist-nested-functions **.babelrc** -```json +```js +// without options { "plugins": ["transform-hoist-nested-functions"] } + +// with options +// NOTE: transform-class-properties is required in order to run the code +{ + "plugins": [ + ["transform-hoist-nested-functions", { + "methods": true + }], + "transform-class-properties" + ] +} ``` ### Via CLI @@ -126,7 +170,7 @@ cd babel-plugin-transform-hoist-nested-functions npm install # ... hackity hack hack ... npm run test:local # Including tests (mocha), code coverage (nyc), code style (eslint), type checks - # (flow) and benchmarks. + # (flow) and benchmarks. ``` See package.json for more dev scripts you can use. diff --git a/src/index.js b/src/index.js index 4377918..402c02b 100644 --- a/src/index.js +++ b/src/index.js @@ -4,6 +4,7 @@ import type { NodePath, Scope } from 'babel-traverse'; class InnerScopeVisitor { referencedScopes: Scope[]; + thisReferencedScopes: Scope[]; ReferencedIdentifier = (path: NodePath) => { const binding = path.scope.getBinding(path.node.name); @@ -20,11 +21,10 @@ class InnerScopeVisitor { let {scope} = path; while (scope && (scope = scope.getFunctionParent())) { // eslint-disable-line no-cond-assign if (!scope.path.isArrowFunctionExpression()) { - // istanbul ignore next: could be initialized elsewhere - if (!this.referencedScopes) { - this.referencedScopes = []; + if (!this.thisReferencedScopes) { + this.thisReferencedScopes = []; } - this.referencedScopes.push(scope); + this.thisReferencedScopes.push(scope); return; } scope = scope.parent; @@ -59,6 +59,12 @@ export default function ({types: t, template}: {types: BabelTypes, template: Bab const declarationTemplate = template(` var NAME = VALUE; `); + const symbolTemplate = template(` + new Symbol(NAME) + `); + const thisMemberReferenceTemplate = template(` + this[METHOD] + `); return { visitor: { Function (path: NodePath) { @@ -72,14 +78,33 @@ export default function ({types: t, template}: {types: BabelTypes, template: Bab // or the global scope. const innerScope = new InnerScopeVisitor(); path.traverse(innerScope); + const thisReferencedScopes = uniqueScopes(innerScope.thisReferencedScopes || []); const referencedScopes = uniqueScopes(innerScope.referencedScopes || []); + const allReferencedScopes = uniqueScopes([ + ...(innerScope.referencedScopes || []), + ...thisReferencedScopes + ]); const targetScope = deepestScopeOf( path, - referencedScopes + allReferencedScopes .concat(path.scope.getProgramParent()) .filter(scope => scope !== path.scope) - ); - if (!targetScope || targetScope === path.scope.parent) { + ); + if (!targetScope) return; + if (targetScope === path.scope.parent) { + if ( + this.opts.methods && + targetScope.path.isClassMethod() && + thisReferencedScopes.indexOf(targetScope) !== -1 && + referencedScopes.indexOf(targetScope) === -1 + ) { + const parentScope: Scope = targetScope.parent; + const containingClassBodyPath: NodePath = targetScope.path.parentPath; + const id = parentScope.generateUidIdentifierBasedOnNode(path.node.id || path.node, 'hoistedMethod'); + parentScope.push({kind: 'const', id, init: symbolTemplate({NAME: t.stringLiteral(id.name)}).expression}); + containingClassBodyPath.unshiftContainer('body', t.classProperty(id, Object.assign({}, path.node, {shadow: true}), true)); + path.replaceWith(thisMemberReferenceTemplate({METHOD: id}).expression); + } return; } if (path.node.id) { diff --git a/test/fixtures/handle-references-to-this/actual.js b/test/fixtures/handle-references-to-this/actual.js index 3cdeb2d..a92a51b 100644 --- a/test/fixtures/handle-references-to-this/actual.js +++ b/test/fixtures/handle-references-to-this/actual.js @@ -5,7 +5,7 @@ class A { method() { - // FIXME: not hoisted but we could make it a "private" method + // NOTE: not hoisted return () => this; } } diff --git a/test/fixtures/handle-references-to-this/expected.js b/test/fixtures/handle-references-to-this/expected.js index 8d4dfed..ba38d70 100644 --- a/test/fixtures/handle-references-to-this/expected.js +++ b/test/fixtures/handle-references-to-this/expected.js @@ -7,7 +7,7 @@ var _this = this; class A { method() { - // FIXME: not hoisted but we could make it a "private" method + // NOTE: not hoisted return () => this; } } diff --git a/test/fixtures/hoist-nested-methods-if-options.methods-true/actual.js b/test/fixtures/hoist-nested-methods-if-options.methods-true/actual.js new file mode 100644 index 0000000..183f986 --- /dev/null +++ b/test/fixtures/hoist-nested-methods-if-options.methods-true/actual.js @@ -0,0 +1,20 @@ +class A { + outer () { + // NOTE: hoisted + (function () {})(); + } +} + +class B { + outer () { + // NOTE: hoisted to bound method + (() => this)(); + } +} + +class C { + static outer () { + // NOTE: hoisted to static method + console.log((() => this)()); + } +} diff --git a/test/fixtures/hoist-nested-methods-if-options.methods-true/expected.js b/test/fixtures/hoist-nested-methods-if-options.methods-true/expected.js new file mode 100644 index 0000000..7f54a12 --- /dev/null +++ b/test/fixtures/hoist-nested-methods-if-options.methods-true/expected.js @@ -0,0 +1,29 @@ +const _hoistedMethod = new Symbol("_hoistedMethod"), + _hoistedMethod2 = new Symbol("_hoistedMethod2"); + +var _hoistedAnonymousFunc2 = function () {}; + +class A { + outer() { + // NOTE: hoisted + _hoistedAnonymousFunc2(); + } +} + +class B { + [_hoistedMethod] = () => this; + + outer() { + // NOTE: hoisted to bound method + this[_hoistedMethod](); + } +} + +class C { + [_hoistedMethod2] = () => this; + + static outer() { + // NOTE: hoisted to static method + console.log(this[_hoistedMethod2]()); + } +} \ No newline at end of file diff --git a/test/fixtures/hoist-nested-methods-if-options.methods-true/options.json b/test/fixtures/hoist-nested-methods-if-options.methods-true/options.json new file mode 100644 index 0000000..7ffe630 --- /dev/null +++ b/test/fixtures/hoist-nested-methods-if-options.methods-true/options.json @@ -0,0 +1,3 @@ +{ + "plugins": [["transform-hoist-nested-functions", {"methods": true}]] +} diff --git a/test/fixtures/not-hoist-mutated-funcs/actual.js b/test/fixtures/not-hoist-mutated-funcs/actual.js index 5007a9d..7090b9b 100644 --- a/test/fixtures/not-hoist-mutated-funcs/actual.js +++ b/test/fixtures/not-hoist-mutated-funcs/actual.js @@ -35,3 +35,19 @@ function inner(param) {} inner.name; })(); + +(class { + outer() { + // FIXME: unsafely hoisted + const inner = () => {}; + inner.someProp = 1; + } +}); + +(class { + outer() { + // NOTE: hoisted to bound method + const inner = () => this.constructor.name; + inner.name; + } +}); diff --git a/test/fixtures/not-hoist-mutated-funcs/expected.js b/test/fixtures/not-hoist-mutated-funcs/expected.js index ffe7683..5d095dd 100644 --- a/test/fixtures/not-hoist-mutated-funcs/expected.js +++ b/test/fixtures/not-hoist-mutated-funcs/expected.js @@ -1,3 +1,5 @@ +const _hoistedMethod = new Symbol("_hoistedMethod"); + (function () { // NOTE: not hoisted function inner(param) {} @@ -50,4 +52,24 @@ _inner4 = function inner(param) {}; var inner = _inner4; inner.name; -})(); \ No newline at end of file +})(); + +var _hoistedAnonymousFunc2 = () => {}; + +(class { + outer() { + // FIXME: unsafely hoisted + const inner = _hoistedAnonymousFunc2; + inner.someProp = 1; + } +}); + +(class { + [_hoistedMethod] = () => this.constructor.name; + + outer() { + // NOTE: hoisted to bound method + const inner = this[_hoistedMethod]; + inner.name; + } +}); \ No newline at end of file diff --git a/test/fixtures/not-hoist-mutated-funcs/options.json b/test/fixtures/not-hoist-mutated-funcs/options.json new file mode 100644 index 0000000..7ffe630 --- /dev/null +++ b/test/fixtures/not-hoist-mutated-funcs/options.json @@ -0,0 +1,3 @@ +{ + "plugins": [["transform-hoist-nested-functions", {"methods": true}]] +} diff --git a/test/fixtures/not-hoist-nested-methods-by-default/actual.js b/test/fixtures/not-hoist-nested-methods-by-default/actual.js new file mode 100644 index 0000000..fe20d8d --- /dev/null +++ b/test/fixtures/not-hoist-nested-methods-by-default/actual.js @@ -0,0 +1,20 @@ +class A { + outer () { + // NOTE: hoisted + (function () {})(); + } +} + +class B { + outer () { + // NOTE: not hoisted (!options.methods) + (() => this)(); + } +} + +class C { + static outer () { + // NOTE: not hoisted (!options.methods) + console.log((() => this)()); + } +} diff --git a/test/fixtures/not-hoist-nested-methods-by-default/expected.js b/test/fixtures/not-hoist-nested-methods-by-default/expected.js new file mode 100644 index 0000000..ec78c3e --- /dev/null +++ b/test/fixtures/not-hoist-nested-methods-by-default/expected.js @@ -0,0 +1,22 @@ +var _hoistedAnonymousFunc2 = function () {}; + +class A { + outer() { + // NOTE: hoisted + _hoistedAnonymousFunc2(); + } +} + +class B { + outer() { + // NOTE: not hoisted (!options.methods) + (() => this)(); + } +} + +class C { + static outer() { + // NOTE: not hoisted (!options.methods) + console.log((() => this)()); + } +} \ No newline at end of file diff --git a/test/fixtures/not-hoist-nested-methods-by-default/options.json b/test/fixtures/not-hoist-nested-methods-by-default/options.json new file mode 100644 index 0000000..48bbbd6 --- /dev/null +++ b/test/fixtures/not-hoist-nested-methods-by-default/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["transform-hoist-nested-functions"] +} diff --git a/test/fixtures/not-hoist-nested-methods-if-options.methods-false/actual.js b/test/fixtures/not-hoist-nested-methods-if-options.methods-false/actual.js new file mode 100644 index 0000000..0717f87 --- /dev/null +++ b/test/fixtures/not-hoist-nested-methods-if-options.methods-false/actual.js @@ -0,0 +1,20 @@ +class A { + outer () { + // NOTE: hoisted + (function () {})(); + } +} + +class B { + outer () { + // NOTE: not hoisted (!options.methods) + (() => this)(); + } +} + +class C { + static outer () { + // NOTE: not hoisted + console.log((() => this)()); + } +} diff --git a/test/fixtures/not-hoist-nested-methods-if-options.methods-false/expected.js b/test/fixtures/not-hoist-nested-methods-if-options.methods-false/expected.js new file mode 100644 index 0000000..3c14a44 --- /dev/null +++ b/test/fixtures/not-hoist-nested-methods-if-options.methods-false/expected.js @@ -0,0 +1,22 @@ +var _hoistedAnonymousFunc2 = function () {}; + +class A { + outer() { + // NOTE: hoisted + _hoistedAnonymousFunc2(); + } +} + +class B { + outer() { + // NOTE: not hoisted (!options.methods) + (() => this)(); + } +} + +class C { + static outer() { + // NOTE: not hoisted + console.log((() => this)()); + } +} \ No newline at end of file diff --git a/test/fixtures/not-hoist-nested-methods-if-options.methods-false/options.json b/test/fixtures/not-hoist-nested-methods-if-options.methods-false/options.json new file mode 100644 index 0000000..8373530 --- /dev/null +++ b/test/fixtures/not-hoist-nested-methods-if-options.methods-false/options.json @@ -0,0 +1,3 @@ +{ + "plugins": [["transform-hoist-nested-functions", {"methods": false}]] +} diff --git a/test/fixtures/react-render-callback/actual.js b/test/fixtures/react-render-callback/actual.js index ebc0420..4c5ae49 100644 --- a/test/fixtures/react-render-callback/actual.js +++ b/test/fixtures/react-render-callback/actual.js @@ -38,7 +38,7 @@ class C { render() { return { - // NOTE: not hoisted + // NOTE: hoisted to bound bethod (val) =>
this.set(val + 1)}> + clicked {val} times +
; + render() { return - { - // NOTE: not hoisted - val =>
this.set(val + 1)}> - clicked {val} times -
} + {this[_hoistedMethod]} ; } diff --git a/test/fixtures/react-render-callback/options.json b/test/fixtures/react-render-callback/options.json index 12f9eca..5d3918b 100644 --- a/test/fixtures/react-render-callback/options.json +++ b/test/fixtures/react-render-callback/options.json @@ -1,3 +1,3 @@ { - "plugins": ["syntax-jsx"] + "plugins": ["syntax-jsx", ["transform-hoist-nested-functions", {"methods": true}]] }