support routeProcess plugins defined in external npm modules

[chrisguttandin]

🧩 Feature request

Description

I would like to propose adding support for external routeProcess plugins. It would be great if their order of execution could be defined in Scully's config file.

Describe the solution you'd like

Currently the documentation for routeProcess plugins says: "There is no need to update the config file for this." But I think it is necessary to import those plugins. Otherwise Scully can't know that there is a routeProcess plugin somewhere in the node_modules folder. I think the following should work to make Scully aware of an external routeProcess plugin.

// scully.config.js
import 'my-external-route-process-plugin';

However, I think the developer experience could be improved a little. Currently the priority of routeProcess plugins is defined when registering the plugin. Since the plugin could be defined in an external node module which could have been developed by someone else it's not immediately clear by looking at Scully's config file which plugin will run first.

Once the order has been figured out by looking up all registerPlugin() calls there is no way for a Scully user to change the order if the plugins have been developed by someone else.

If I'm not mistaken routeProcess plugins are the only plugins which can be prioritized like this. All other plugins seem to be executed in the same order in which they appear in the config file.

Therefore I would like to propose adding a new entry to the config file for configuring route process plugins. It could look somehow like this:

// scully.config.js
import { myExternalRouteProcessPlugin } from  'my-external-route-process-plugin';
import { anotherExternalRouteProcessPlugin } from  'another-external-route-process-plugin';

export const config = {
    routeProcessPlugins: [myExternalRouteProcessPlugin, anotherExternalRouteProcessPlugin]
};

This new routeProcessPlugins entry would work similar as the defaultPostRenderers entry. It would allow the users of Scully to define routeProcess plugins and to re-order them as it works best for their use case. And I think it would also bring the behavior of routeProcessPlugins more inline with all other types of plugins.

Describe alternatives you've considered

As mentioned above it's currently possible to import an external routeProcess plugin with a "side-effects" import statement. However re-defining their priority is not possible so far.

[SanderElias]

~What is the added value to this? You can already as many as you like. The 4rth parameter of registerplugin is a number. That number sets the order.~ Ok, rereading your question. I see your issue now. If this is a common concern we will address this.

[aaronfrost]

@SanderElias the value would be that PersonA could make a plugin and deploy it. In their NPM package they call the registerPlugin function and register it with the 4th param.

Then PersonB comes a long and they can use that person's plugin and set their own priority instead of just getting the priority of the person who built it.

[chrisguttandin]

I think @SanderElias is right. It's a theoretic problem so far. I only discovered this since I was implementing route process plugin support for angular-prerender. I don't have an actual use case where this would be a problem.

However if you want this to be changed before any user runs into the problem I would be happy to provide a PR. Although I'm not quite sure how to implement this in a backwards compatible way. Maybe each route process plugin that gets registered but isn't listed in the config file could log a warning for now.

[SanderElias]

@chrisguttandin I think the best way would be to enable a way to change the registration number. The actual sort happens just before they are executed. So a setPluginPriotity function would do the trick. There is a symbol for the priority attached to the plugin. something as:

const plugin = findPlugin('myPluginName')
plugin[routeProcessPriority]=newNumber

Would do the trick. We might need to add the export for routeProcessPriority

I would welcome a PR for this.

[chrisguttandin]

@SanderElias I created a basic PR (#1329) but I wasn't sure where to put a test for the new function. Please let me know if I should add a test and where to put it.

And please let me know if there is anything else I need to change.

[SanderElias]

@chrisguttandin Thanks! A test would be welcome. It should be added to our jest tests in ./tests/jest/src/__tests__ Just a no-fuss test would be ok, and checking the error if you try to set an unexisting one should be enough.

registerPlugin('routeProcess','one', async (x) => x, 100);
registerPlugin('routeProcess','two', async (x) => x, 200)
// some actual testing goes here.

[SanderElias]

This is done and merged