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

[Experimental] Flip startTransition and isPending #7838

Closed
wants to merge 5 commits into from
Closed

[Experimental] Flip startTransition and isPending #7838

wants to merge 5 commits into from

Conversation

phaux
Copy link

@phaux phaux commented Apr 30, 2021

@timdorr
Copy link
Member

timdorr commented May 1, 2021

Is this released anywhere yet? I know it's an unstable API, so things can change whenever they want. But just so folks using this don't run into trouble.

@phaux
Copy link
Author

phaux commented May 2, 2021

It's already released as latest experimental version. The last version that works with react-router-dom@experimental is {react,react-dom}@0.0.0-experimental-78120032d.

@damikun
Copy link

damikun commented May 8, 2021

This is not yet part of React router... @phaux what kind of brunch is that? it is not in dev or v6 yet... Just I'm a bit confused... Is that brunch as Experimental even working looks like has a lot of issues...

I used Link extension so far...

xport interface LinkProps
  extends Omit<React.AnchorHTMLAttributes<HTMLAnchorElement>, "href"> {
  replace?: boolean;
  state?: State;
  to: To;
  transitionTime?: number
  onTransitionStateChange?: (state: boolean) => void;
}

export const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(
  function LinkWithRef(
    {
      onClick,
      onTransitionStateChange,
      replace: replaceProp = false,
      state,
      target,
      to,
      transitionTime = 5000,
      ...rest
    },
    ref
  ) {
    let href = useHref(to);
    let navigate = useNavigate();
    let location = useLocation();
    let path = useResolvedPath(to);

    const [startTransition, isPending] = unstable_useTransition({
      busyDelayMs: transitionTime ,
    });

    useEffect(() => {
      onTransitionStateChange && onTransitionStateChange(isPending);
    }, [isPending]);

    function handleClick(event: React.MouseEvent<HTMLAnchorElement>) {
      !isPending &&
        startTransition(() => {
          if (onClick) onClick(event);
          if (
            !event.defaultPrevented && // onClick prevented default
            event.button === 0 && // Ignore everything but left clicks
            (!target || target === "_self") && // Let browser handle "target=_blank" etc.
            !isModifiedEvent(event) // Ignore clicks with modifier keys
          ) {
            event.preventDefault();

            // If the URL hasn't changed, a regular <a> will do a replace instead of
            // a push, so do the same here.
            let replace =
              !!replaceProp || createPath(location) === createPath(path);

            navigate(to, { replace, state });
          }
        });
    }

    return (
      // eslint-disable-next-line jsx-a11y/anchor-has-content
      <a
        {...rest}
        href={href}
        onClick={handleClick}
        ref={ref}
        target={target}
      />
    );
  }
);

function isModifiedEvent(event: React.MouseEvent) {
  return !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);
}

@RichardLindhout
Copy link

There are even more changes unstable_ prefixes got removed

@damikun
Copy link

damikun commented Jun 3, 2021

@RichardLindhout What I know transition is still unstable_ other one gets stable with lastest React but transition not.. Only if this changed in last days...

@RichardLindhout
Copy link

In the experimental release I'm importing straight from React.useTransition but I'm using ^0.0.0-experimental-46491dce9 maybe they've changed it back to unstable

@damikun
Copy link

damikun commented Jun 3, 2021

Are you using typescript and last Types? I remember i was updating types month ago for this... But maybe I should check correctly again :P But good to know the import is directly available...

@phaux
Copy link
Author

phaux commented Jun 3, 2021

Updated React again. react-test-rendered is not the latest because it fails.

@RichardLindhout
Copy link

Are you using typescript and last Types? I remember i was updating types month ago for this... But maybe I should check correctly again :P But good to know the import is directly available...

Types were not up to date yet I think. Btw React 18 is now released so hope react-router will update soon :)

@timdorr
Copy link
Member

timdorr commented Jun 9, 2021

Can we bump this to the @alpha releases, now that they are out? That should be a more stable target. Thanks!

@phaux
Copy link
Author

phaux commented Jun 10, 2021

Updated and now it warns about not using ReactDOM.createRoot, but when I change it then it fails. react-test-renderer still fails with alpha version so I left it at last experimental version that works.

@timdorr
Copy link
Member

timdorr commented Jun 10, 2021

Did you use React.createRoot().render()? That's the new API (you create a root and then render or hydrate it)

@phaux
Copy link
Author

phaux commented Jun 11, 2021

Did you use React.createRoot().render()? That's the new API (you create a root and then render or hydrate it)

Yes, like that:

 act(() => {
-  ReactDOM.render(
+  ReactDOM.createRoot(node).render(
     <Router initialEntries={['/home']}>
       <Routes>
         <Route path="home" element={<Home />} />
         <Route path="about" element={<About />} />
       </Routes>
     </Router>,
-    node,
   );
 };

And I get error:

/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:1543
    return doc.body;
               ^

TypeError: Cannot read property 'body' of null
    at getActiveElement (/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:1543:16)
    at getActiveElementDeep (/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:8017:17)
    at getSelectionInformation (/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:8050:21)
    at prepareForCommit (/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:10500:26)
    at commitBeforeMutationEffects (/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:21737:27)
    at commitRootImpl (/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:25273:45)
    at commitRoot (/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:25168:5)
    at finishConcurrentRender (/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:24511:9)
    at performConcurrentWorkOnRoot (/home/lis/code/react-router/node_modules/react-dom/cjs/react-dom.development.js:24389:5)
    at workLoop (/home/lis/code/react-router/node_modules/react-dom/node_modules/scheduler/cjs/scheduler.development.js:253:34)
    at flushWork (/home/lis/code/react-router/node_modules/react-dom/node_modules/scheduler/cjs/scheduler.development.js:226:14)
    at Immediate.performWorkUntilDeadline (/home/lis/code/react-router/node_modules/react-dom/node_modules/scheduler/cjs/scheduler.development.js:516:21)
    at processImmediate (internal/timers.js:461:21)

@mjackson mjackson closed this Jul 23, 2021
@mjackson mjackson deleted the branch remix-run:dev-experimental July 23, 2021 22:22
brophdawg11 added a commit that referenced this pull request Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants