Hook destructuring optimization (#8381)

* Add hook destructuring optimization

* oops, accidentally included loose mode

* inline hook destructuring optimization plugin

* fix test nesting

* fix lockfile

* allow any react hook

* Add page to stats-app with hooks

packages/next/build/babel/plugins/optimize-hook-destructuring.ts

+import { PluginObj } from '@babel/core'
+import { NodePath } from '@babel/traverse'
+import * as BabelTypes from '@babel/types'
+
+// matches any hook-like (the default)
+const isHook = /^use[A-Z]/
+
+// matches only built-in hooks provided by React et al
+const isBuiltInHook = /^use(Callback|Context|DebugValue|Effect|ImperativeHandle|LayoutEffect|Memo|Reducer|Ref|State)$/
+
+export default function({
+  types: t,
+}: {
+  types: typeof BabelTypes
+}): PluginObj<any> {
+  const visitor = {
+    CallExpression(path: NodePath<BabelTypes.CallExpression>, state: any) {
+      const onlyBuiltIns = state.opts.onlyBuiltIns
+
+      // if specified, options.lib is a list of libraries that provide hook functions
+      const libs =
+        state.opts.lib &&
+        (state.opts.lib === true
+          ? ['react', 'preact/hooks']
+          : [].concat(state.opts.lib))
+
+      // skip function calls that are not the init of a variable declaration:
+      if (!t.isVariableDeclarator(path.parent)) return
+
+      // skip function calls where the return value is not Array-destructured:
+      if (!t.isArrayPattern(path.parent.id)) return
+
+      // name of the (hook) function being called:
+      const hookName = (path.node.callee as BabelTypes.Identifier).name
+
+      if (libs) {
+        const binding = path.scope.getBinding(hookName)
+        // not an import
+        if (!binding || binding.kind !== 'module') return
+
+        const specifier = (binding.path.parent as BabelTypes.ImportDeclaration)
+          .source.value
+        // not a match
+        if (!libs.some(lib => lib === specifier)) return
+      }
+
+      // only match function calls with names that look like a hook
+      if (!(onlyBuiltIns ? isBuiltInHook : isHook).test(hookName)) return
+
+      path.parent.id = t.objectPattern(
+        path.parent.id.elements.map((element, i) =>
+          t.objectProperty(t.numericLiteral(i), element)
+        )
+      )
+    },
+  }
+
+  return {
+    name: 'optimize-hook-destructuring',
+    visitor: {
+      // this is a workaround to run before preset-env destroys destructured assignments
+      Program(path, state) {
+        path.traverse(visitor, state)
+      },
+    },
+  }
+}

packages/next/build/babel/preset.ts

@@ -116,6 +116,13 @@ module.exports = (
           property: 'createElement',
         },
       ],
+      [
+        require('./plugins/optimize-hook-destructuring'),
+        {
+          // only optimize hook functions imported from React/Preact
+          lib: true,
+        },
+      ],
       require('@babel/plugin-syntax-dynamic-import'),
       require('./plugins/react-loadable-plugin'),
       [

test/.stats-app/pages/hooks.js

+import React, { useState, useCallback } from 'react'
+
+export default () => {
+  const [clicks1, setClicks1] = React.useState(0)
+  const [clicks2, setClicks2] = useState(0)
+
+  const doClick1 = React.useCallback(() => {
+    setClicks1(clicks1 + 1)
+  }, [clicks1])
+
+  const doClick2 = useCallback(() => {
+    setClicks2(clicks2 + 1)
+  }, [clicks2])
+
+  return (
+    <>
+      <h3>Clicks {clicks1}</h3>
+      <button onClick={doClick1}>Click me</button>
+
+      <h3>Clicks {clicks2}</h3>
+      <button onClick={doClick2}>Click me</button>
+    </>
+  )
+}

test/unit/next-babel.test.js

@@ -28,71 +28,93 @@ const babel = (code, esm = false) =>
   }).code
 
 describe('next/babel', () => {
-  it('should transform JSX to use a local identifier in modern mode', () => {
-    const output = babel(`const a = () => <a href="/">home</a>;`, true)
-
-    // it should add a React import:
-    expect(output).toMatch(`import React from"react"`)
-    // it should hoist JSX factory to a module level variable:
-    expect(output).toMatch(`var __jsx=React.createElement`)
-    // it should use that factory for all JSX:
-    expect(output).toMatch(`__jsx("a",{href:"/"`)
-
-    expect(
-      babel(`const a = ()=><a href="/">home</a>`, true)
-    ).toMatchInlineSnapshot(
-      `"import React from\\"react\\";var __jsx=React.createElement;var a=function a(){return __jsx(\\"a\\",{href:\\"/\\"},\\"home\\");};"`
-    )
-  })
+  describe('jsx-pragma', () => {
+    it('should transform JSX to use a local identifier in modern mode', () => {
+      const output = babel(`const a = () => <a href="/">home</a>;`, true)
 
-  it('should transform JSX to use a local identifier in CommonJS mode', () => {
-    const output = babel(trim`
-      const a = () => <React.Fragment><a href="/">home</a></React.Fragment>;
-    `)
-
-    // Grab generated names from the compiled output.
-    // It looks something like this:
-    //   var _react = _interopRequireDefault(require("react"));
-    //   var __jsx = _react["default"].createElement;
-    // react: _react
-    // reactNamespace: _react["default"]
-    const [, react, reactNamespace] = output.match(
-      /(([a-z0-9_]+)(\[[^\]]*?\]|\.[a-z0-9_]+)*?)\.Fragment/i
-    )
-
-    expect(output).toMatch(`var ${reactNamespace}=`)
-    expect(output).toMatch(`require("react")`)
-    expect(output).toMatch(`var __jsx=${react}.createElement`)
-    // Fragment should use the same React namespace import:
-    expect(output).toMatch(`__jsx(${react}.Fragment`)
-    expect(output).toMatch(`__jsx("a",{href:"/"`)
-
-    expect(babel(`const a = ()=><a href="/">home</a>`)).toMatchInlineSnapshot(
-      `"\\"use strict\\";var _interopRequireDefault=require(\\"@babel/runtime-corejs2/helpers/interopRequireDefault\\");var _react=_interopRequireDefault(require(\\"react\\"));var __jsx=_react[\\"default\\"].createElement;var a=function a(){return __jsx(\\"a\\",{href:\\"/\\"},\\"home\\");};"`
-    )
-  })
+      // it should add a React import:
+      expect(output).toMatch(`import React from"react"`)
+      // it should hoist JSX factory to a module level variable:
+      expect(output).toMatch(`var __jsx=React.createElement`)
+      // it should use that factory for all JSX:
+      expect(output).toMatch(`__jsx("a",{href:"/"`)
+
+      expect(
+        babel(`const a = ()=><a href="/">home</a>`, true)
+      ).toMatchInlineSnapshot(
+        `"import React from\\"react\\";var __jsx=React.createElement;var a=function a(){return __jsx(\\"a\\",{href:\\"/\\"},\\"home\\");};"`
+      )
+    })
+
+    it('should transform JSX to use a local identifier in CommonJS mode', () => {
+      const output = babel(trim`
+        const a = () => <React.Fragment><a href="/">home</a></React.Fragment>;
+      `)
+
+      // Grab generated names from the compiled output.
+      // It looks something like this:
+      //   var _react = _interopRequireDefault(require("react"));
+      //   var __jsx = _react["default"].createElement;
+      // react: _react
+      // reactNamespace: _react["default"]
+      const [, react, reactNamespace] = output.match(
+        /(([a-z0-9_]+)(\[[^\]]*?\]|\.[a-z0-9_]+)*?)\.Fragment/i
+      )
+
+      expect(output).toMatch(`var ${reactNamespace}=`)
+      expect(output).toMatch(`require("react")`)
+      expect(output).toMatch(`var __jsx=${react}.createElement`)
+      // Fragment should use the same React namespace import:
+      expect(output).toMatch(`__jsx(${react}.Fragment`)
+      expect(output).toMatch(`__jsx("a",{href:"/"`)
 
-  it('should support Fragment syntax', () => {
-    const output = babel(`const a = () => <>hello</>;`, true)
+      expect(babel(`const a = ()=><a href="/">home</a>`)).toMatchInlineSnapshot(
+        `"\\"use strict\\";var _interopRequireDefault=require(\\"@babel/runtime-corejs2/helpers/interopRequireDefault\\");var _react=_interopRequireDefault(require(\\"react\\"));var __jsx=_react[\\"default\\"].createElement;var a=function a(){return __jsx(\\"a\\",{href:\\"/\\"},\\"home\\");};"`
+      )
+    })
 
-    expect(output).toMatch(`React.Fragment`)
+    it('should support Fragment syntax', () => {
+      const output = babel(`const a = () => <>hello</>;`, true)
 
-    expect(babel(`const a = () => <>hello</>;`, true)).toMatchInlineSnapshot(
-      `"import React from\\"react\\";var __jsx=React.createElement;var a=function a(){return __jsx(React.Fragment,null,\\"hello\\");};"`
-    )
+      expect(output).toMatch(`React.Fragment`)
+
+      expect(babel(`const a = () => <>hello</>;`, true)).toMatchInlineSnapshot(
+        `"import React from\\"react\\";var __jsx=React.createElement;var a=function a(){return __jsx(React.Fragment,null,\\"hello\\");};"`
+      )
+    })
+
+    it('should support commonjs', () => {
+      const output = babel(
+        trim`
+        const React = require('react');
+        module.exports = () => <div>test2</div>;
+      `,
+        true
+      )
+
+      expect(output).toMatchInlineSnapshot(
+        `"var React=require('react');var __jsx=React.createElement;module.exports=function(){return __jsx(\\"div\\",null,\\"test2\\");};"`
+      )
+    })
   })
 
-  it('should support commonjs', () => {
-    const output = babel(
-      trim`
-      const React = require('react');
-      module.exports = () => <div>test2</div>;
-    `,
-      true
-    )
-
-    expect(output).toMatchInlineSnapshot(
-      `"var React=require('react');var __jsx=React.createElement;module.exports=function(){return __jsx(\\"div\\",null,\\"test2\\");};"`
-    )
+  describe('optimize-hook-destructuring', () => {
+    it('should transform Array-destructured hook return values use object destructuring', () => {
+      const output = babel(
+        trim`
+        import { useState } from 'react';
+        const [count, setCount] = useState(0);
+      `,
+        true
+      )
+
+      expect(output).toMatch(trim`
+        var _useState=useState(0),count=_useState[0],setCount=_useState[1];
+      `)
+
+      expect(output).toMatchInlineSnapshot(
+        `"import{useState}from'react';var _useState=useState(0),count=_useState[0],setCount=_useState[1];"`
+      )
+    })
   })
 })