Pnp-modern-search: Race condition when trying to load custom renderers via application extension

[jhholm]

Version used
3.10 & 3.12.1

Describe the bug
When using custom renderers via an application extension it is possible that the Search Results webpart tries to load the registered renderers before the application extension has registered them. If this happens, you are not able to see any custom renderers in the property pane.

In most cases the application extension seems to able to register the renderers before the search results webpart tries to load them. This seems to be because of default modern SharePoint page loading pattern as application extensions are usually executed before page webparts.

My use case, where I can reproduce this issue easily, is using a library component to provide React components for the custom renderer. In this case the "usual" modern SharePoint page loading pattern breaks as the extension has to load an external javascript file. Search Results webpart seems to try to load the registered renderers before they are registered by the application extension. You can also easily "simulate" this situation in your custom renderer by adding a 10 second delay before registering the renderer by putting the following code before registering the renderer via ResultService.

await new Promise(resolve => setTimeout(resolve, 10000));

This bug is not reproducable if you already have working custom renderers. The issue can only be reproduced within a tenant/site collection that does not have a single custom renderer registered. If you already have a successfully registered renderer and you can see it in the Search Results webpart property pane, you are not able to reproduce this.

To make testing easier, you can easily simulate registration via your browsers console as the registration is using global scope variables.

const renderer = {id: "foobar", name: "foobar", icon: "OfficeLogo"};
const SEARCH_RENDERERS_OBJECT_NAME = "pnp-spfx-search-renderers";= "pnp-spfx-search-renderers";
if (window[SEARCH_RENDERERS_OBJECT_NAME] === undefined) {
  window[SEARCH_RENDERERS_OBJECT_NAME] = [];
}
window[SEARCH_RENDERERS_OBJECT_NAME].push(renderer);

If you already have successfully registered custom renderers that are showing in your property pane, you are able to see a new dummy renderer icon.

To Reproduce
Steps to reproduce the behavior:

If you have any custom renderers already active, this is not reproducable.

1. Create a new site collection
2. Add the PnP Modern Search - Search Web Parts app to your site
3. Add the Search results webpart to a page
4. Do one of the following
   a) Add a custom renderer that has a 10 second delay
   b) Run the following code snippet above to simulate a renderer registration after the page has been fully loaded
5. Notice that you are not able to see any custom renderers in your webpart property pane

Expected behavior
You should be able to have custom renderers that take some time to load.

Desktop (please complete the following information):
-Chrome 81.0.4044.138

How to fix this issue
The issue seems to be here. getRegisteredRenderers is called before registerRenderer when using a delayed custom renderer or a custom renderer with a library component. In that case the global scope variable is still undefined and Search Results webpart code gets the undefined object. As registerRender gets called later on, the global scope variable is set as an array and then the renderer is pushed to the array. Search Results webpart still keeps to undefined object.

If registerRenderer is called before getRegisteredRenderers, the Search Result Webpart will use the array object and any mutations done to that object afterwards.

By changing getRegisteredRenderers to instantiate the global scope variable to an empty array if it is undefined, the webpart will get all mutations to that array correctly and not be dependent that registerRenderer is called first. This is also the reason why this issue is not reproducable if you already have working custom renderers.

I forked the repository and created a branch with the fix above.
See the commit here.

For an example on creating a "delayed" custom renderer see here

[wobba]

We will change the custom rendering logic and start using SPFx libraries for this going forward. This ensures we load it from the webpart, and we avoid this issue alltogether. So expect the current custom rendering option to go away at some point.

When supported, we can implement library discovery based on an interface and convention as well, and you won't have to know the library id to load it - which will be a dependency for v1.

Feel free to commit a PR with your change.

[jhholm]

l'll create a PR, sure.

If you are moving towards using Extensibility libraries, will this change require using web components via handlebars templates? The main reason why I haven't changed to use the library component way is that the process requires manual work by editing handlebars templates.

[jhholm]

PR #265

[FranckyC]

Resolved by #265

[stact]

Hi guys,

I've still the same issue with September 2020 version. I tried also to install May 2020 version to check if no regression.

I'm compiled the package from https://github.com/microsoft-search/pnp-modern-search/tree/main/search-custom-renderer

gulp bundle
gulp package solution

The register seems to be initialized correctly:

    @override
    public onInit(): Promise<void> {
        this._resultService = new ResultService();
        this.onChangeHappened.bind(this);
        this._resultService.registerRenderer(this.componentId, 'CodeRenderer', 'QueryList', this.onChangeHappened, ['Subheader']);
        return Promise.resolve();
    }

I'm using new fresh demo tenant.

@jhholm does PR #265 fix the issue on your side?

Let me know if you need environment information (in private message)!

[wobba]

As we're moving away from this approach and over to web components we will not prioritize if this is still an issue. If someone solves it and PR's, we will of course accept.

[jhholm]

I am on a forked 3.10 version with this fix and it works. Haven't tested on newer versions since the PR. In general the approach is not optimal, so there might be some other issues related to it.

[stact]

Thanks @jhholm, I've tried the forked solution still got the problem ^^ in fact other issues could be related to this approach.

It's not a problem, I will find a workaround to implement our needs to be working (custom template).

Let's wait for the new custom rendering logic. @wobba is the solution available somewhere to test this approach?

Thanks again guys for your help, we really appreciate your availability!

[wobba]

Latest version support web components which you can install and use. See https://microsoft-search.github.io/pnp-modern-search/search-extensibility-library/getting-started/

We'll advance this in the v4 branch.

[stact]

@wobba really thank you! the Extensibility Library is powerful! It's working fine.

Will v4 have the scalability to allow pre-configured models? (to include this custom web component)
It might be useful to have a reusable presentation of the results for non-developer users.

Thanks again for this great tool

[wobba]

Custom rendering is really a developer scenario. If people contribute modules to the projects we can surely host and release them :) The idea would not to have one lib with everything, but you can install what you need. v4 will support loading the component by id, so multiple ones will be supported, yes.