RFC: Expose shallow flag in router event callbacks

[bragle]

Goal

Provide an easy way to determine if a request is shallow in the router events API

Background

My team and I has implement a page loader on one of our sites, and it works, but we want to disable the loader on shallow requests. I searched around for a bit and stumbled upon #3322, which is the same issue as the one we're having.

Proposal

See pull request #13243 for proposed implementation.

I believe the easiest way to implement this is to add whether or not the route is shallow to the event callbacks.

There are a couple of ways this could be implemented. Here's one example of how it can be done, and how I've implemented it in my PR.

Example 1:

By converting the callback parameter as to an object, it would be easy to find the parameter needed, and it would also be expandable, should there be any need to add more parameters to the route events in the future.

Using the callback function with this solution would look something like this:

router.events.on('routeChangeStart', (request) => {}); // request = { as: 'example.com', shallow: true }

Example 2:

This is the way I've implemented it in my PR. The main reason why I've done this is for backwards compatibility.

Using the callback function with this solution would look something like this:

router.events.on('routeChangeStart', (as, shallow) => {}); // as = 'example.com', shallow = true

The only alternative way to solve this that I can think of is to do routing "manually" (without the next router) or create some kind of wrapper for it, which, in my opinion, would be overly complicated for such an easy solution.

---

I have not written an RFC before, so feedback on how I could improve it for next time is greatly appreciated!

[banerjeesouvik]

I think we should expect an object instead of boolean as second parameter. In that case other config or properties can be added (if required) in future without increasing the length of the parameters.

router.events.on('routeChangeStart', (as, {shallow}) => {}); // as = 'path' shallow = true

[bragle]

That's true. I'm gonna go ahead and update my PR to this unless someone has an even better idea on how to solve it!

[matepaiva]

I also think this is a better approach. Also, I think shallow could have a default value as false, to be retro compatible.

[bragle]

Yep, my implementation does make shallow false by default.

[pueyo5]

I'm using "routeChangeComplete" for marketing pageview events and I want to exclude shallow requests. This feature would be awesome.

[banerjeesouvik]

This is how I am handling a similar situation in my app -

import { parse } from 'url'
....
....

handleRouteChangeStart(url) {

    this.oldPath = window.location.pathname
}

handleRouteChangeComplete(url) {

    const newPath = parse(url).pathname

    // Check if the pathname changes
    if (this.oldPath !== newPath) {

      // Trigger pageView event
    }
}

[bragle]

I honestly forgot about this, but I can create a new pull request when I have some spare time. I don't think there's any way to solve this at the moment, but the change required to get this functionality is minimal 😸

[pueyo5]

By now I'm going to use @banerjeesouvik solution, but it would be nice to have this functionality.
BTW, why is the PR closed? Wasn't it a good solution?

[bragle]

I'm actually not sure why it was closed, but I suspect that it's because I created this RFC after the PR. I'll make a new one when I have time and we'll see what happens 😺

[axelg12]

@bragle was there any update on this or the new PR?

[bragle]

For anyone that might be waiting: I have created another PR (#19802) for this issue 😸

[bragle]

Good news everyone! The PR got merged, so we're getting this feature 🎉