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

SearchForm's autoload and cause lots of XHR requests #700

Open
ScopeyNZ opened this issue Oct 18, 2018 · 9 comments
Open

SearchForm's autoload and cause lots of XHR requests #700

ScopeyNZ opened this issue Oct 18, 2018 · 9 comments

Comments

@ScopeyNZ
Copy link
Contributor

Steps to reproduce

  • Spin up a copy of cwp/cwp-recipe-kitchen-sink
  • Open your network tab
  • Browse to an edit page
  • Observe the SearchForm requests:

lotsofsearchforms

In this case, there's a bunch of searchable gridfields added by the comments module.

Ideally these are only loaded if you click on the advanced filters dropdown. I imagine there's a very small number of times people browse to edit a page and intend to use advanced search on a gridfield.

@maxime-rainville
Copy link
Contributor

The file search doesn't do that, because the search component isn't actually rendered until you click the search toggle.

The GridField and SiteTree search always render their search components, but hide them with CSS. A simple fix could be to do the same thing as the File Search.

A side effect of this is that if you untoggle the search in File, the search component is completely destroyed and you loose whatever search term you had in there. You'll also get a Form Schema request each time you toggle the search. #627 Should fix this.

The GridField search remember its term when you untoggle it.

Another solution could be to only render <SearchForm /> when you actually toggle the advanced search. But that would mean you would have fetch the form schema each time you open the advanced search, until we implement schema caching.

@ScopeyNZ
Copy link
Contributor Author

ScopeyNZ commented Nov 1, 2018

You'll also get a Form Schema request each time you toggle the search. #627 Should fix this.

CCers recently added a refetchSchemaOnMount prop for FormBuilderLoader. Have you tried toggling that (to false)?

I think the attached issue was more along the lines of caching schema per object type - so separate forms can use the same cache. Redux-form should remember individual(ly named) forms.

@ScopeyNZ
Copy link
Contributor Author

You'll also get a Form Schema request each time you toggle the search. #627 Should fix this.

CCers recently added a refetchSchemaOnMount prop for FormBuilderLoader. Have you tried toggling that (to false)?

Just adding to this - I've had a lot of experience recently with not reloading form schema between un/re-mounts so I'm happy to provide assistance to anyone who picks this up - @Scopey on community slack

@lekoala
Copy link
Contributor

lekoala commented Feb 26, 2019

It would be great if these calls would only be made once someone actually clicks on the search icon, because currently if you have a couple of gridfields it makes a lot of calls that are not necessary most of the time.
Just realised that on one of my page that takes ages to load for no good reason. And the new GridFieldLazyLoader is not helping here either.

@GuySartorelli
Copy link
Member

I've noticed as well that with a lazy-loading gridfield, the searchform gets loaded twice - once when the main edit form is loaded, and again when the gridfield gets loaded.

@lekoala
Copy link
Contributor

lekoala commented May 24, 2022

in the meantime, for those who don't need search, using this

$gridfield->getConfig()->removeComponentsByType(GridFieldFilterHeader::class);

will remove the calls. I'm not sure there will be a fix anytime soon for this issue since it's deeply related to how the field is implemented.

lazy loading could help here as well, but it's not really implemented in a way that is compatible as far as I can see.

@lekoala
Copy link
Contributor

lekoala commented Feb 27, 2024

So here I come again, since i've been working on another project, and got the exact same issue. Example:

image

I was thinking that maybe caching the schema would help, but that's not the slow part. The slow part is that each ajax calls needs to instantiate the whole form in order to instantiate the GridFieldFilterHeader component. The whole form, for one component, each time. So, the more gridfield you have, the slower it gets. Exponentially (well, not technically, but you get the idea...).

At this stage, I think there is no good solution : components SHOULD NOT call something in order to render themselves. Delaying the graphql call is only a half baked solution: the blazing fast "react" form will depend on a very slow ajax calls that renders the whole form in php in order to produce the schema. And since the schema may include state, well... caching is not your friend there.

What is really strange, is that the schema is already bundled on render by the php side

https://github.com/silverstripe/silverstripe-framework/blob/c2b606c24cb7c123ab3b0228202d89661cdc29c2/src/Forms/GridField/GridFieldFilterHeader.php#L446-L448

But somehow there is this in the js ... (i guess that's whats causing the reload, but i'm not sure)

I may be missing something, but why is there a need to refetch on mount when the initial state could be provided on rendering ?

@lekoala
Copy link
Contributor

lekoala commented Mar 1, 2024

a minor improvement is to add this in the init method

    public function init()
    {
        parent::init();

        // Optimize search form
        if (strpos($this->getRequest()->getURL(), 'schema/SearchForm') !== false) {
            session_write_close();
        }
    }

i'm not sure the schema endpoint should depend on the session and if they do, they probably never update it so no need to lock the xhr request behind a session lock

@GuySartorelli
Copy link
Member

That hints at a much wider problem with the way Silverstripe framework handles sessions which the hybrid sessions module resolves - Silverstripe framework locks your session for the entirety of each request, and won't accept any more requests from you (i.e. using the same browser window) while processing your request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants