inertiajs / inertia

Inertia.js lets you quickly build modern single-page React, Vue and Svelte apps using classic server-side routing and controllers.
https://inertiajs.com
MIT License
6.02k stars 405 forks source link

Improvement on router #1889

Closed allanmcarvalho closed 2 weeks ago

allanmcarvalho commented 1 month ago

Versions:

Describe the problem:

In sometimes we need to update url without reload the page and the method replaceState has protected visibility. After package built we can use it, but cause a lint error. An example of use: I have a product search with "s" query attribute, and every time that user type a new search I would like to put the new sentence on "s" to preserve share url and the navigation historic.

I would create a pull request for this, but I confess that I don't know about Js enough. So I believe it would be more costly to make adjustments to my code than for someone with knowledge to quickly correct it if they think it's worth it.

Steps to reproduce:

The code above works, but cause lint error

import {toRaw} from "vue";
import {router} from "@inertiajs/vue3";

let page = toRaw(this.$page);
page.url = route('foo_bar', {s: "new search"});
router.replaceState(page)
peter-emad99 commented 1 month ago

try using router.reload ( and with input debouncing of course)


router.reload({
data:{ s:'new search;}
replace:true 
});
 if you don't want to keep all the search in history put the replace option to false
allanmcarvalho commented 1 month ago

@peter-emad99 doing this, inertia performs requests to server. The intention is only update the current page state URL shares or page refresh pressing F5 for example.

For a small project, maybe this work, but in a large a lot of unnecessary requests will be made.

peter-emad99 commented 4 weeks ago

for manipulating the parameters client side only i use these two helpers (but be aware that could keep the url state un synched with the server)


export const removeQueryParameter = (parameter: string) => {
    let url = new URL(window.location.toString());
    url.searchParams.delete(parameter);
    history.replaceState(null, "", url.href);
};
export const setQueryParameter = (parameter: string, value: string) => {
    const url = new URL(window.location.toString());
    url.searchParams.set(parameter, value);
    history.replaceState(null, "", url.href);
};```
RobertBoes commented 4 weeks ago

What would be the benefit of replacing state in this example? You'd need to make a request at some point, so if you just replace the state with a query parameter and not actually execute the search then the state wouldn't be in sync with what the user is doing, right?

Why not just use a debounce, trigger the search and then that would also update the URL. It's pretty much the same thing that's done on the Inertia demo; https://demo.inertiajs.com/organizations. Although there the search action is performed immediately, but you could easily do a debounce

allanmcarvalho commented 4 weeks ago

for manipulating the parameters client side only i use these two helpers (but be aware that could keep the url state un synched with the server)



export const removeQueryParameter = (parameter: string) => {

    let url = new URL(window.location.toString());

    url.searchParams.delete(parameter);

    history.replaceState(null, "", url.href);

};

export const setQueryParameter = (parameter: string, value: string) => {

    const url = new URL(window.location.toString());

    url.searchParams.set(parameter, value);

    history.replaceState(null, "", url.href);

};```

I tried this, but it cause a bug when you need to navigate on history due a missing inertia page configs. Don't is a like a charm approach!

allanmcarvalho commented 4 weeks ago

What would be the benefit of replacing state in this example? You'd need to make a request at some point, so if you just replace the state with a query parameter and not actually execute the search then the state wouldn't be in sync with what the user is doing, right?

Why not just use a debounce, trigger the search and then that would also update the URL. It's pretty much the same thing that's done on the Inertia demo; https://demo.inertiajs.com/organizations. Although there the search action is performed immediately, but you could easily do a debounce

If search be on same server and you need to get informations on db, yes! But if you use Scout with Algolia? This is my case, do requests to server is like spend unnecessary resources.

RobertBoes commented 4 weeks ago

If search be on same server and you need to get informations on db, yes! But if you use Scout with Algolia? This is my case, do requests to server is like spend unnecessary resources.

I don't see how that would be any different tho? You're saying you use Laravel Scout, so you'd go to the backend regardless. You can do the exact same, with a partial reload, as an example;

Route::get('/search', function (Request $request) {
    return Inertia::render('Search', [
        'orders' => fn () => $request->search ? null : Order::search($request->search)->get(),
    ]);
});
allanmcarvalho commented 4 weeks ago

If search be on same server and you need to get informations on db, yes! But if you use Scout with Algolia? This is my case, do requests to server is like spend unnecessary resources.

I don't see how that would be any different tho? You're saying you use Laravel Scout, so you'd go to the backend regardless. You can do the exact same, with a partial reload, as an example;


Route::get('/search', function (Request $request) {

    return Inertia::render('Search', [

        'orders' => fn () => $request->search ? null : Order::search($request->search)->get(),

  ]);

});

When using algolia to show an index, you don't interact with application data directly (on your server). Algolia search get data directly from Algolia server. So when I need update searched query on url (to preserve share context), I don't need do any request to my servers. Let's think that we have 10k clients doing a search on my site at same time, will be 10k, 20k, 30k unnecessary requests made to my server. I don't know what's your point.

RobertBoes commented 4 weeks ago

When using algolia to show an index, you don't interact with application data directly (on your server). Algolia search get data directly from Algolia server. So when I need update searched query on url (to preserve share context), I don't need do any request to my servers.

Well, you mentioned you used Scout, so that's what I'm answering.

Let's think that we have 10k clients doing a search on my site at same time, will be 10k, 20k, 30k unnecessary requests made to my server. I don't know what's your point.

I feel like now you're just looking to argue, since the numbers you're talking are enormous, but let's say that is the case. With algiolia you have 10k searches per month for free, after that you'll start paying $0.50 for every 1k requests. You mention it's at the same time, so let's say within an hour? That'll come down to a huge amount of requests, say the average would then be 1k requests per hour per day. That would sum up to ~750.000 requests, which is about $375.000, which you'd need to pay for. If you'd take the backend approach, you can easily cache that search query, so you can probably stay under the 10k limit, or at least it would likely filter out quite a few duplicate queries, which would end up saving you an enormous amount of money.


Anyway, local visits aren't a thing in Inertia, and I don't think it'll arrive anytime soon. The router.replaceState() method is protected for a reason, it's an internal method that shouldn't be called externally. Likely the biggest reason for that, if a method is public it would need to be supported, so if one uses the method incorrectly that would result in issues/questions, so yeah, the method shouldn't be used, or you should ignore/silence the warnings and not expect any support if that method changes behaviour.

You can also opt to ignore this method and do it yourself, by interacting with the history API directly;

const url = new URL(window.location);
url.searchParams.set('s', 'abc');
history.replaceState(window.history.state, '', url.href);

(note, that's just an example, haven't tested this)

allanmcarvalho commented 4 weeks ago

Anyway, local visits aren't a thing in Inertia, and I don't think it'll arrive anytime soon. The router.replaceState() method is protected for a reason, it's an internal method that shouldn't be called externally. Likely the biggest reason for that, if a method is public it would need to be supported, so if one uses the method incorrectly that would result in issues/questions, so yeah, the method shouldn't be used, or you should ignore/silence the warnings and not expect any support if that method changes behaviour.

Yeah, you are right. This make sense.

I feel like now you're just looking to argue, since the numbers you're talking are enormous, but let's say that is the case. With algiolia you have 10k searches per month for free, after that you'll start paying $0.50 for every 1k requests. You mention it's at the same time, so let's say within an hour? That'll come down to a huge amount of requests, say the average would then be 1k requests per hour per day. That would sum up to ~750.000 requests, which is about $375.000, which you'd need to pay for. If you'd take the backend approach, you can easily cache that search query, so you can probably stay under the 10k limit, or at least it would likely filter out quite a few duplicate queries, which would end up saving you an enormous amount of money.

The cost is my north, so maybe I need to think more about this and then make a decision about what to do.

But thanks for you opinion.

peter-emad99 commented 3 weeks ago

I tried this, but it cause a bug when you need to navigate on history due a missing inertia page configs. Don't is a like a charm approach!

can you explain more what the bug it causes when using this approach ?

driesvints commented 2 weeks ago

Seems @RobertBoes provided some insights into this one so closing this one.