Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hooks, useImperativeMethods and multiple refs #14072

Closed
latviancoder opened this issue Nov 2, 2018 · 16 comments
Closed

Hooks, useImperativeMethods and multiple refs #14072

latviancoder opened this issue Nov 2, 2018 · 16 comments

Comments

@latviancoder
Copy link

@latviancoder latviancoder commented Nov 2, 2018

I am not sure where to post this question, feel free to delete it if it doesn't belong here.

Here is a small Accordion with AccordionPanels. It uses hooks to manage state and when some panel is opened we need to scroll to it, so we need a ref.

My question is: Is this how hooks are supposed to be used? Do you see some problems with this approach?

App.js

function App() {
    const panels = [
        'Panel 1',
        'Panel 2',
        'Panel 3'
    ];

    const [currentIndex, onClick, refs] = useAccordion(panels.length);

    return <Accordion>
        {panels.map((panel, index) => (
            <AccordionPanel
                ref={refs[index]}
                key={index}
                label={panel}
                isOpen={currentIndex === index}
                onClick={() => onClick(index)}
            />
        ))}
    </Accordion>;
}

Accordion.js

function useAccordion(panelsCount) {
    const [currentIndex, setCurrentIndex] = useState(undefined);
    let refs = {};

    for (let i = 0; i <= panelsCount; i++) {
        refs[i] = createRef();
    }

    useEffect(() => {
        if (currentIndex !== undefined) {
            refs[currentIndex].current.scrollTo();
        }
    }, [currentIndex]);

    function onClick(newIndex) {
        setCurrentIndex(currentIndex === newIndex ? undefined : newIndex);
    }

    return [currentIndex, onClick, refs];
}

const AccordionPanel = React.forwardRef((props, ref) => {
    const containerRef = useRef();

    useImperativeMethods(ref, () => ({
        scrollTo: () => console.info('do scrolling')
    }));

    return <div onClick={props.onClick} ref={containerRef}>
        {props.label}, {props.isOpen && 'open'}
    </div>;
});

function Accordion(props) {
    return <div>{props.children}</div>;
}
@gaearon
Copy link
Member

@gaearon gaearon commented Nov 2, 2018

The part that looks shady to me is this:

    let refs = {};

    for (let i = 0; i <= panelsCount; i++) {
        refs[i] = createRef();
    }

That should, I think, happen only once. useRef() would make more sense to me but it doesn't allow lazy initialization.

@sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Nov 2, 2018

A workaround for lazy init is:

let ref = useRef();
if (ref.current == null) {
  ref.current = ...;
}

(Edited to use ref.current not ref.)


Edit from @gaearon: see also #14490 (comment) for a potentially nicer pattern.

@gaearon
Copy link
Member

@gaearon gaearon commented Nov 2, 2018

You mean ref.current right?

@latviancoder
Copy link
Author

@latviancoder latviancoder commented Nov 2, 2018

Is it possible to use useRef inside of loops?

@sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Nov 2, 2018

@gaearon Sorry, edited.

@latviancoder
Copy link
Author

@latviancoder latviancoder commented Nov 3, 2018

So is putting refs inside of state a good idea? This is the only way I managed to put it inside of useEffect. Otherwise it doesn't seem to work.

const [refs, setRefs] = useState(undefined);

useEffect(() => {
    let refs = {};
    for (let i = 0; i <= panelsCount; i++) {
        refs[i] = createRef();
    }
    setRefs(refs);
}, []);

@gaearon
Copy link
Member

@gaearon gaearon commented Nov 3, 2018

Hmm sounds complicated. Just refs alone should work. Mind posting this as a code sandbox?

@latviancoder
Copy link
Author

@latviancoder latviancoder commented Nov 3, 2018

@monsterkrampe
Copy link

@monsterkrampe monsterkrampe commented Nov 6, 2018

I created a fork of the sandbox:
https://codesandbox.io/s/p55llrmooj

This does not solve the original problem, that we can't really have an array of refs.
But it does seem like a clean workaround to me at least for this particular usecase.
I'm happy to receive feedback on this.

@Jessidhia
Copy link
Contributor

@Jessidhia Jessidhia commented Nov 8, 2018

This doesn't preserve old refs if panelsCount changes (they'll be reattached after the current render commits), but you can use useMemo for this:

const refs = React.useMemo(() => 
  Array.from(
    { length: panelsCount },
    () => React.createRef()
  ),
  [panelsCount]
)

@sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Dec 11, 2018

There are really good reasons why hooks can't be used in loops and this seems to try to be clever and workaround it but will inevitably only hit the same problems. The solution is to create a separate component for each items so that each item can have separate state (with keys, not index!).

This leaves the problem that you might require two passes to figure out which the currentIndex is. That's what the "Call/Return" thing is supposed to solve but it doesn't exist yet so the best you can do is the two passes.

@Jessidhia
Copy link
Contributor

@Jessidhia Jessidhia commented Dec 11, 2018

How do you envision writing this without lifting state to the parent component?

Activating one of the children implies de-activating its siblings.

Although the side-effect of scrolling to the child can be done by the child themselves (Element#scrollIntoView).

@sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Dec 12, 2018

I guess there is currently no true idiomatic way of dealing with sets of refs. The issue is that the set itself grows or shrinks between updates but the true value changes in the commit phase as a side-effect. I think for most cases, you just want to change the set as a side-effect too. You can do that simply by mutating a persistent Map.

If you're dealing with raw children and want a ref to each item in the list, you should use the React.Children helpers to create a list where each item has a unique key.

Then you can attach a ref to that which stores the result in a Map based on that key.

let refs = useRef(new Map()).current;

let childrenWithKeys = React.Children.toArray(children);
return childrenWithKeys.map(child =>
  React.cloneElement(child, {ref: inst => inst === null ? refs.delete(child.key) :  refs.set(child.key, inst)})
);

If it is a custom data structure like the example above then you can do the same thing but that data structure needs a key as part of it:

const panels = [
    {key: 'a', name: 'Panel 1' },
    {key: 'b', name: 'Panel 2' },
    {key: 'c', name: 'Panel 3' },
];

let refs = useRef(new Map()).current;

return panels.map(panel =>
  <AccordionPanel
      ref={inst => inst === null ? refs.delete(panel.key) : refs.set(panel.key, inst)}
      key={panel.key}
      label={panel.name}
      isOpen={currentKey === panel.key}
      onClick={() => onClick(panel.key)}
  />
);

@gaearon
Copy link
Member

@gaearon gaearon commented Jan 16, 2019

Regarding lazily initializing useRef, our recommendation is here: #14490 (comment).

@gaearon gaearon closed this as completed Jan 16, 2019
@gaearon
Copy link
Member

@gaearon gaearon commented Jan 17, 2019

@WebTravel
Copy link

@WebTravel WebTravel commented Mar 28, 2019

@gaearon I can't uderstand why I have to use useImperativeHandle hook when I use forwardRef in my functional component.
For instance I have something parent component, when I create ref and send this ref to children component:

export const SliderWithTabs = ({
  sliderItems,
  ...
}) => {
   const slidesCollection = useRef(null);
   .......
  <Tabs
       items={sliderItems}
       ref={slidesCollection}
    />
};

And I return array of refs from children components

export const Tabs = forwardRef(
  ({ items }, ref) => {
    const tabRef = items.map(() => useRef(null));

    useImperativeHandle(ref, () => tabRef.map(({ current }) => current));

    return (
      <Fragment>
        <TabsList>
          {items.map(({ key, title }, index) => (
            <TabsItem
              ref={tabRef[index]}
              key={key}
            >
              {title}
            </TabsItem>
          ))}
        </TabsList>
      </Fragment>
    );
  }
);

Example above works for me! But I can't understand why I can not use forward ref without useImperativeHandle. Next Example doesn't work:

export const SliderWithTabs = ({
  sliderItems,
  ...
}) => {
   const slidesCollection = sliderItems.map(() => useRef(null));
   .......
  <Tabs
       items={sliderItems}
       ref={slidesCollection}
    />
};

And I return array of refs from children components

export const Tabs = forwardRef(
  ({ items }, ref) => {
    return (
      <Fragment>
        <TabsList>
          {items.map(({ key, title }, index) => (
            <TabsItem
              ref={ref[index]}
              key={key}
            >
              {title}
            </TabsItem>
          ))}
        </TabsList>
      </Fragment>
    );
  }
);

When children component mount I can get my array of refs and this will be [{current: null}, {current: null} ...]. But next component update I get just null instead array of refs. And I see I have to use useImperativeHandle always. But I don't understand why I get null instead array of refs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants