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

Proposal: chainRoute helper #10

Closed
Kelin2025 opened this issue Mar 23, 2022 · 8 comments
Closed

Proposal: chainRoute helper #10

Kelin2025 opened this issue Mar 23, 2022 · 8 comments
Assignees
Labels
enhancement Improvement of existing functionality

Comments

@Kelin2025
Copy link
Collaborator

Kelin2025 commented Mar 23, 2022

Problem

Whenever some route is opened, we often want to do some work before showing actual page

For example,

const postsRoute = createRoute()

sample({
  clock: postsRoute.opened,
  target: getPostsFx
})

const PostsPage = () => {
  const isLoading = useStore(getPostsFx.pending)
  if (isLoading) { /* ... */ }
  /* ... */
}

Yes, you can implement this logic yourself by using basic effector operators.

However, there're two problems.

First-of-all, it's not unified. So each project will implement it differently. Also, we can't build some helpful wrappers around that on the library side

And then, there's lot of boilerplate.
You have to create pending stores, update it, hide page contents, etc.
It might look easy on simple examples. But if you need to do more than a single request, or do some checks like session validation - that's a lot of work

Solution

Introducing chainRoute!

chainRoute is a helper, that creates a virtual clone of passed route and listens to passed events before open

Example:

import { createRoute, chainRoute } from 'atomic-router'

const editBotRoute = createRoute<{ botId: string }>()

const loadedRoute = chainRoute({
  route: editBotRoute,
  beforeOpen: getBotFx.prepend(({ params: { botId } }) => ({ id: botId })),
  openOn: getBotFx.doneData,
  cancelOn: getBotFx.failData
})

What happens here

  1. Create loadedRoute
  2. Listen to route.opened / route.updated
  3. Store current route.$params and route.$query
  4. Trigger beforeOpen
  5. When either cancelOn or route.left is triggered during request, we reset stored params/query
  6. When openOn is triggered, we compare route.$params/$query with stored ones. And, if they are equal, we:
    7.1. Trigger loadedRoute.open with stored params
    7.2. Reset stored params
  7. When route.left is triggered and loadedRoute.$isOpened is true, we also trigger loadedRoute.left

Notes

So many stuff here! Why not accept just route and effect?

Unfortunately, that won't be flexible.

With just effect you cannot solve the following cases:

  • Multiple consequent requests
  • Non-fetch based loading (e.g. through websocket)
  • Custom logic (e.g. session check - skip it if it's already checked, token refresh etc)

in a declarative way.

So we'll give a bit more low-level control over that, so users can implement any logic they want

What are the benefits over sample series?

Like I said at the start, there's a lot of boilerplate required for a manual handling
But moreover, if we have consequent requests or custom logic, we need even more extra code:

For example:

const getUserFx = createEffect(...)
const getUserPostsFx = createEffect(...)

// We need to write this stuff to hide page during load
const $isLoading = every({
  stores: [getUserFx.pending, getUserPostsFx.pending]
  predicate: Boolean
})

With chainRoute you can rely on returned route:

const loadedRoute = chainRoute({
  route: mainRoute,
  beforeOpen: getUserFx,
  openOn: getUserPostsFx.doneData,
  cancelOn: [getUserFx.failData, getUserPostsFx.failData]
})

// And just use loadedRoute.$isOpened to display the contents

Some sugar, please!

Sure.

// Use already existing route instead of creating new one
chainRoute({
  route: mainRoute,
  /* ... */
  chainedRoute: loadedRoute
})
// Duplicate route (just for fun)
const chainedRoute = chainRoute(mainRoute)
// Pass only `beforeEnter` effect 
// to use its `doneData/failData` as `openOn/cancelOn`
const loadedRoute = chainRoute({
  route: mainRoute,
  beforeOpen: getUserFx
})
// Multiple triggers (like clock in `sample`)
const loadedRoute = chainRoute({
  route: mainRoute,
  beforeOpen: [cleanupForm, getUserFx],
  openOn: [
    guard({ clock: getUserFx.doneData, filter: ... })
  ],
  cancelOn: [
    getUserFx.failData, 
    guard({ clock: getUserFx.doneData, filter: ... })
  ]
})

Purpose of this thread

Before I officially publish this solution, I'd like to share it with users, so we could discuss

  • Can it solve all the needed cases
  • Can we do better
  • Do we need extra API
  • or, y' know, think of betters namings etc

So yeah, feel free to write your thoughts on that!

@Kelin2025 Kelin2025 pinned this issue Mar 23, 2022
@Kelin2025
Copy link
Collaborator Author

Use case: authorized helper

By using chainRoute we can do higher-level wrappers as well.

import { guard, createEvent } from 'effector'
import { chainRoute, RouteInstance, RouteParamsAndQuery } from 'atomic-router'

const authorizedRoute = <Params>(route: RouteInstance<Params>) => {
  const sessionCheckStarted = createEvent<RouteParamsAndQuery<Params>>()

  const alreadyAuthorized = guard({
    clock: sessionCheckStarted,
    filter: $isActuallyAuthorized,
  })

  guard({
    clock: sessionCheckStarted,
    filter: $isNotAuthorized,
    target: showLoginFormModal
  })

  return chainRoute({
    route,
    beforeEnter: sessionCheckStarted,
    enterOn: [alreadyAuthorized, sessionEstablished],
  })
}

const userRoute = createRoute()
const authorizedUserRoute = authorized(userRoute)

Here we have two scenarios

  • If user is already authorized, we open the route immediately
  • If not, we display login form modal and wait until sessionEstablished to actually show the page

And then we can do another chain from authorizedUserRoute to load more data

@Kelin2025 Kelin2025 added the enhancement Improvement of existing functionality label Mar 23, 2022
@Kelin2025 Kelin2025 self-assigned this Mar 25, 2022
@Kelin2025 Kelin2025 mentioned this issue Mar 27, 2022
@Kelin2025
Copy link
Collaborator Author

Kelin2025 commented Mar 28, 2022

We also need onCancel parameter, which will be triggered when open is cancelled by cancelOn events (but not if user leaves original route)
Then we could use it like this

chainRoute({
  route: editPostRoute,
  beforeOpen: fetchPostFx,
  // These are optional but I set them to make it more obvious
  openOn: fetchPostFx.doneData,
  cancelOn: fetchPostFx.failData,
  // ===
  onCancel: split({
    match: error => error.status,
    cases: { 
      404: redirect({ route: notFoundRoute }),
      403: redirect({ route: notAllowedRoute })
    }
  }),
  chainedRoute: loadedRoute
})

So here we:

  1. Open editPostRoute
  2. Trigger fetchPostFx
  3. If fetchPostFx.doneData is triggered, we open loadedRoute
  4. If fetchPostFx.failData is triggered, we redirect to notFoundRoute notAllowedRoute depending on the received error

The only problem is that I don't like naming hell here. onCancel <=> cancelOn similarity + onCancel might confuse users because they can treat "leaving the original route during fetching" as a "manual cancelling"

@kirillku
Copy link

kirillku commented Mar 29, 2022

Hi, the proposed solutions feels to me a huge overhead because a lot of new things are introduced. Also, you have to handle edge cases already (adding onCancel and you will need to add more).

This is what I feel should be possible with a guarded route:

  1. Do nothing
  2. Redirect to some other route
  3. Show some other content without changing the url
  4. Show loading while page data is loaded (ability to unify loading for the project)

With suggested solution, it's not clear what to do if I want to show some content (for example 404 page or login page) but keep the url.


I would suggest to instead extend route with filter:

// 1. Do nothing
// If user is not authenticated, route will not match and nothing will happen
const myRoute = createRoute({ filter: $isAuthenticated });

// 2. Redirect to some other route
// Achieved by creating an inverted route
const myRoute = createRoute({ filter: $canEnter });
const myInvertedRoute = createRoute({ filter: $canEnter.map(v => !v) });

sample({
  source: myInvertedRoute.opened,
  target: myRoute.open
});

// 3. Show some other content without changing the url
// If user is not authenticated, login route will match and login page shown
const homeRoute = createRoute({ filter: $isAuthenticated });
const loginRoute = createRoute({ filter: isAuthenticated.map(v => !v) });

// 4. Show loading while page data is loaded
// When any of the effects are pending, match `globalLoadingRoute` and show full screen spinner
const getUserFx = createEffect();
const $user = restore(getUserFx.doneData, null);

const mainRoute = createRoute();
const loadedRoute = createRoute({ filter: $user.map(Boolean) });
const globalLoadingRoute = createRoute({ filter: every([geUserFx.pending, ...], true) });

sample({
  source: mainRoute.opened,
  target: getUserFx,
});

@Kelin2025
Copy link
Collaborator Author

@kirillku yeah and you skipped a lot of work that chainRoute does under the hood

  • You already have boilerplate code in business code to open the 2nd route
  • You don't close 2nd route when the original one gets closed
  • You don't handle cases like "original route closed during mid-fetching"
  • No information on how all the routes in your example will be opened/closed

I agree that chainRoute API is kinda huge, but otherwise you just write all of it manually in the business code

@Kelin2025
Copy link
Collaborator Author

Also, "just filtering" has another problem.

If something tries to open blocked route, it'd be just skipped.
No information on who tried to open it, no way to handle it.
So you'll handle it separately, still creating ton of boilerplate code.

@kirillku
Copy link

kirillku commented Mar 30, 2022

@Kelin2025 could you please describe what do you want from this feature, like in addition to my points. Seems I have a different concept in my mind. What are the requirements and expected behavior for a gated route?

(the proposed approach reminds me react-router@3 a bit because they had to reinvent react lifecycle in there)

@Kelin2025
Copy link
Collaborator Author

Kelin2025 commented Apr 4, 2022

chainRoute solves this scenario

  1. User opens the original route
  2. We start some fetching or checks
  3. When we're done and if the route is still opened, we open the subroute
  4. When user closes the original route, we close the subroute as well

in a unified way instead of manually handling "should I open subroute" situations etc.

I took a look on your filters approach - combined with patronum/every it looks almost the same

This thing

const mainRoute = createRoute()

sample({
  clock: mainRoute.opened,
  fn: ({ params: { botId } }) => ({ id: botId }),
  target: getBotFx,
})

const botLoadedRoute = createRoute({
  filter: every({
    predicate: Boolean,
    stores: [
      mainRoute.$isOpened,
      $bot.map(bot => bot && !bot.enabled)
    ],
  }),
})

Will work the same as

const mainRoute = createRoute()

const botLoadedRoute = chainRoute({
  route: mainRoute,
  beforeOpen: getBotFx,
  openOn: sample({
    clock: getBotFx.doneData,
    filter: (bot) => !bot.enabled,
  }),
  cancelOn: [
    getBotFx.failData,
    sample({
      clock: getBotFx.doneData,
      filter: (bot) => bot.enabled,
    }),
  ],
})

The second one does even look more verbose

However, the first approach works only with stores.
Which means that, if you have some logic that doesn't store anything, you still need to create boolean store and connect it to something. And if here we can just "check and go", with stores we have to "check, store true and reset to false before next time"


Authorized route example with filters will look like this:

const authorizedRoute = <Params>(route: RouteInstance<Params>) => {
  guard({
    clock: route.opened,
    filter: $isNotAuthorized,
    target: showLoginFormModal
  })

  return createRoute({
    filter: every({
      predicate: Boolean,
      stores: [route.$isOpened, $isActuallyAuthorized] 
    })
  })
}

IMO this solution

  • looks less intuitive
  • how do we pass params to subroute
  • if we have to re-check session on each open, it will go closed -> opened route -> opened authorizedRoute -> reset boolean store -> closed authorizedRoute -> check stuff -> open authorizedRoute back, so we need to write extra workarounds to get closed -> opened route -> check stuff -> open authorizedRoute

@Kelin2025
Copy link
Collaborator Author

@Kelin2025 Kelin2025 unpinned this issue Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing functionality
Projects
None yet
Development

No branches or pull requests

2 participants