Migration guide for replacing RootCloseWrapper?

[acroyear]

I have a really old project that we're trying to bring up to date. We use (and derive from) RootCloseWrapper from 0.6.2. The new version uses a hook useRootClose which I don't have a problem with other than, well, what's the best way to migrate? There's no documentation that I can find for that.

Also, the hook isn't the best solution for us because a lot of our code is still in class-based form. Is it possible to basically create a 'dumb' replacement of RootClassWrapper by just using a simple functional component with the hook and then render props.children inside a fragment?

[taion]

You can shim this over yourself, no? Create your own component that itself uses useRootClose, and have it do the relevant ref handling.

[ericgio]

@acroyear something like the following:

const RootClose = ({ children, onRootClose, ...props }) => {
  const [rootElement, attachRef] = useState(null);
  useRootClose(rootElement, onRootClose, props);
  return children(attachRef);
};

Usage:

<RootClose
  disabled={disabled}
  onRootClose={() => { /* Do something on root close */ }}>
  {(ref) => (
    <div ref={ref}>{...}</div>
  )}
</RootClose>

@taion: Would you guys be open to a PR for something like this? Seems useful for a lot of people.

[taion]

Sounds great to me. Do we have a migration guide doc in this repo? If not, perhaps something like https://github.com/react-bootstrap/react-bootstrap/blob/master/www/src/pages/migrating.mdx / https://react-bootstrap.github.io/migrating/

[ericgio]

Oh sorry, I meant a PR for an actual component :)

[taion]

I'm not sure... @jquense do you have thoughts here? I don't think we want to encourage still using the old API, but I haven't thought through all the implications

[ericgio]

Fwiw, using a component instead of a hook still seems valid for class-based components, as the OP notes. The main issue with the old RootCloseWrapper was that it relied on findDOMNode.

[jquense]

Yeah, I don't think we want to support both API's. Esp since the component wrapper version if very simple to write. I'd suggest we put the example in the useRootClose hook and see if folks are content with that

[acroyear]

I'd prefer it as just a migration guide entry. A retired API should stay retired. I had no problem with retiring it nor the reasons why. I had a problem that I have old code and no time to come up with the best way to upgrade it to the next API, partly because I had a LOT of other issues at the same time with other libraries I was upgrading.

in particular I had a bunch of unit tests that stopped working because shallow() ceased to work. After all that, I still had unit tests failing because even with mount(), having the root component be functional was causing the underlying DOM to not be accessible for my tests. At this point, I'm not 100% sure that even using this will get past that problem, but we'll see.

[christianh25]

I looked at the migration guide and could not find anything regarding this change. Would it be possible to provide a code sample for the migration? We are using 'RootCloseWrapper ' everywhere in our code and we want to update the package to have the new functionalities. This breaking change is preventing us from upgrading.

[jquense]

@christianh25 read through the tests in the source here for examples. there is now useRootWrapper it is a hook version of the old component and basically functions the same way.