microsoft-search / pnp-modern-search

Home of PnP Modern Search solutions, helping you move from classic to modern SharePoint and beyond
https://microsoft-search.github.io/pnp-modern-search
Other
388 stars 341 forks source link

[Extensibility] Unable to load web components if more than one module is exported from library component #4001

Open michaelmaillot opened 2 months ago

michaelmaillot commented 2 months ago

Version used Ex: 4.13.1

Describe the bug

The current implementation of SPFx Library Component consumption doesn't allow to have more than one exported class.

If the main index.ts file exports something else than the class which implements the IExtensibilityLibrary interface, it won't be loaded by the extensibility service.

To Reproduce Detailed steps to reproduce the behavior:

  1. Create an SPFx library component
  2. Declare a class called HelloLibrary that will implement the IExtensibilityLibrary interface
  3. Add a custom Web Component
  4. Add a React Component called MyReactComponent
  5. Update the main index.ts file to export both HelloLibrary and MyReactComponent
  6. Ship the Bundle & Package Solution of the library component, update it to the tenant and enable it globally
  7. Create a SharePoint Online page and add a PnP Modern Search - Search Results webpart
  8. Update webpart properties to add a reference to the Web Component declared in the library component
  9. Update webpart properties to add a reference to the deployed library component (extensibility libraries, page 4)

Expected behavior

Web Component should be loaded and displayed in the PnP Modern Search - Search Results webpart

Desktop (please complete the following information):

Additional context

First: a huge THANK YOU for this extensibility feature, it allows us (developers) to enhance PnP Modern Search properly without having to inject unsafe JS code! Kudos to the team!

After digging a bit into the Extensibility service, I think I know why it fails. It seems that the loadExtensibilityLibraries method (in the ExtensibilityService.ts file) doesn't check if the loaded exported module is defined during the parsing. As it goes through all the exported modules and as there's more than one, it crashes during the prototype methods check, resulting into not importing the expected module.

My suggestion is to update the code here like this:

const libraryMainEntryPoints = Object.keys(extensibilityLibraryComponent).filter(property => {

    // Return the library main entry point object by checking the prototype methods. They should be matching the IExtensibilityLibray interface.
    const extensibilityLibraryPrototype: IExtensibilityLibrary = extensibilityLibraryComponent[property].prototype;
    return property.indexOf('__') === -1 && extensibilityLibraryPrototype !== undefined && (
        extensibilityLibraryPrototype.getCustomSuggestionProviders ||
        extensibilityLibraryPrototype.getCustomWebComponents ||
        extensibilityLibraryPrototype.getCustomLayouts ||
        extensibilityLibraryPrototype.registerHandlebarsCustomizations ||
        extensibilityLibraryPrototype.getCustomQueryModifiers ||
        extensibilityLibraryPrototype.invokeCardAction);
});

Like this, we also check if the current extensibilityLibraryPrototype in the filter array returns an object during the cast and if not, it will go the next occurence but in the end, it will load the one declared as expected in the library component.

This will allow to leverage library components at its best to centralize reusable React components, shared third-party libraries and custom Web Components.

wobba commented 2 months ago

My suggestion is you do a PR with changes needed to get this working :)

michaelmaillot commented 2 months ago

That was my further request, but I just wanted to get an approval first 😄

You can assign this to me if there's such process in the repo.