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

Support a callback function for unstable_viewTransition props #11716

Open
wants to merge 2 commits into
base: v6
Choose a base branch
from

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Jun 24, 2024

Quick first pass at supporting <Link to="/path" unstable_viewTransition={fn}> where fn is (path: Path) => boolean) - allowing applications to lazily compute the boolean value at click time. The primary use case is large applications where routes are managed in a central manifest - and they need to look up a route in the manifest to know if view transitions should be enabled. This can get prohibitively expensive to do path-matching and looking up routes at render time for all rendered links.

I also want to look at supplying matches to the callback but that's a bit more involved from an implementation standpoint since we need to move the callback execution down into the guts of the router after Fog of War matching has completed.

^ I looked into this and it's totally doable, but it feels a bit odd to do this "use location/matches to determine whether to apply a view transition" at the call-site - that feels far more like a global API. If you want this "generic" approach you would probably just end up using your own Link wrapper to put this callback on every Link in the app. I'm now leaning towards leaving the Link prop as a boolean (targeted call-site version) and adding a new top-level function you can use globally if you prefer to operate off a manifest:

// I know which paths I want to enable for
<Link to="/path" unstable_viewTransition>

// This feels weird - I give you a specific path but then write a very generic function 
// to determine if the destination route wants view transitions
<Link to="/path" unstable_viewTransition={({ location, matches })} => {
  return enableViewTransition(location, matches);
}}>

// This feels like a more appropriate spot for that API
let router = createBrowserRouter(routes, {
  unstable_viewTransition({ location, matches }) {
    return enableViewTransition(location, matches);
  }
});

// Or, even better - why not just give one generic spot to lazily provide navigation opts:
let router = createBrowserRouter(routes, {
  onNavigation({ location, matches, navigateOpts }) {
    return {
      // Use whatever opts were specified at the call site
      ...navigateOpts,
      // But globally determine view transition behavior
      unstable_viewTransition: enableViewTransition(location, matches),
    };
  }
});

Copy link

changeset-bot bot commented Jun 24, 2024

🦋 Changeset detected

Latest commit: 76707b0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
react-router-dom Minor
react-router Minor
react-router-dom-v5-compat Minor
react-router-native Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@brophdawg11 brophdawg11 changed the base branch from dev to v6 July 16, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant