smart-rename for React forwardRef (+ remove layer of indirection on destructuring assignment)

[0xdevalias]

It would be cool if smart-rename was able to handle React's forwardRef:

• https://react.dev/reference/react/forwardRef#forwardref

  • forwardRef(render)

  • Call forwardRef() to let your component receive a ref and forward it to a child component

import { forwardRef } from 'react';

const MyInput = forwardRef(function MyInput(props, ref) {
  // ...
});

Based on that example, we can see that the 2nd param of the React component passed to the forwardRef function is the ref being forwarded; which we can then use within the smart-rename.

---

Looking at my sample webpack'd code (Ref), after using the CLI to unpack it to ./496-unpacked, and then unminify it to ./496-unminified:

⇒ cd ./unpacked/_next/static/chunks

⇒ npx @wakaru/unpacker 496.js -o ./496-unpacked
# ..snip..

⇒ npx @wakaru/unminify ./496-unpacked/* -o ./496-unminified
# ..snip..

Looking at module-10604.js, we can see that it's a React component using forwardRef:

Unpacked:

var r = require(39324),
  // ..snip..
  l = require(70079),
  // ..snip..

exports.Z = l.forwardRef(function (e, t) {
  var n = e.name,
    i = e.placeholder,
    u = e.type,
    c = e.displayName,
    h = e.onChange,
    g = e.onBlur,
    m = e.value,
    p = e.saveOnBlur,
    v = e.icon,
    x = e.onInputIconClick,
    b = e.className,
    y = e.autoComplete,
    w = e.autoFocus,
    j = e.onPressEnter,
  // ..snip..
});

Unminified:

// ..snip..
const l = require(70079);

const { useState, useCallback, useEffect } = l;
// ..snip..

export const Z = l.forwardRef((e, t) => {
  const {
    name,
    placeholder,
    type,
    displayName,
    onChange,
    onBlur,
    value,
    saveOnBlur,
    icon,
    onInputIconClick,
    className,
    autoComplete,
    autoFocus,
    onPressEnter,
  } = e;
  // ..snip..
});

If we had a smart-rename for this, then we could rename e to eProps (or just props), and t to tForwardedRef (or just forwardedRef), and the unminified output could look something more like this:

// ..snip..
const l = require(70079);

const { useState, useCallback, useEffect } = l;
// ..snip..

export const Z = l.forwardRef((eProps, tForwardedRef) => {
  const {
    name,
    placeholder,
    type,
    displayName,
    onChange,
    onBlur,
    value,
    saveOnBlur,
    icon,
    onInputIconClick,
    className,
    autoComplete,
    autoFocus,
    onPressEnter,
  } = eProps;
  // ..snip..
});

This could be improved even further if we removed the layer of indirection on the destructuring assignment on the import of l, as well as the React component props e (which might be a bug/edge case limitation of smart-inline, as I would have expected it to do it)

With that, the output could look more like:

// ..snip..
const { useState, useCallback, useEffect, forwardRef } = require(70079);
// ..snip..

export const Z = forwardRef(
  (
    {
      name,
      placeholder,
      type,
      displayName,
      onChange,
      onBlur,
      value,
      saveOnBlur,
      icon,
      onInputIconClick,
      className,
      autoComplete,
      autoFocus,
      onPressEnter,
    },
    tForwardedRef
  ) => {
    // ..snip..
  }
);

(Note: I'm not 100% sure if we can safely fold the l.forwardRef into the import like I showed in my above example, as it doesn't seem to use the (0, l.useEffect) style call pattern.. but as far as how it would be used in real world code.. my above example is fairly canonical)

[pionxzh]

which might be a bug/edge case limitation of smart-inline, as I would have expected it to do it

This is not a bug. Currently, we don't have a corresponding rule for it. un-indirect-call might be the closest one, but it only focuses on the (0, l.forwardRef) call.

I was worrying about the syntactic correctness of doing this (this issue). It's more like I was struggling with choosing "correctness-first" or "readability-first". But I believe it's a good idea to implement such a rule. We can simply mark it as "unsafe". Will take some time on this after resolving most bugs.