feature: Configure external plugins/controls from main Origo configuration

[sweco-sedalh]

Enables configuring external plugins/controls from the main Origo configuration

This means that you can setup Origo like so:

var origo = new Origo(
  'index.json',
  {
    controls: {
      lmsearch: Lmsearch,
      // lazy imports are also supported
      swiper: async () => (await import("swiper-plugin")).default,
      // etc.
    }
  }
);

And then add their configuration just like the built-in controls:

{
  "controls": [
    {
      "name": "lmsearch",
      "options": {
        "title": "Sökresultat",
        // etc.
      }
    }
  ],
  // ...rest of the configuration...
}

This makes it possible to create a single HTML-file with all possible plugins you may want to use, and then reuse that same HTML-file for various configurations which might be managed by an external tool.

A quick note on the chosen design: This solution does not support plugins that require special setup (like the OIDC plugin). It was initially considered to have a separate array ("plugins") rather then adding these to "controls" as well, however it makes a lot of sense to have them in the same place (from the POV of the configuration it doesn't really matter if a control is built-in or supplied by a plugin, and this way we can re-use most control setup functionality from the existing code).

[tonnyandersson]

@sweco-sedalh Nice addition!

Can you please also submit an issue for this? For reference, transparency and traceability purposes, if nothing else. We usually like to go Issue → Discussion → PR accepted/rejected → PR.

[sweco-sedalh]

Done: #2121

[steff-o]

I don't really understand when and how to use to lazy load stuff. No doubt that you know what you are doing and why, but I can not figure it out.

Can you explain to me the rationale behind the complex lazy loading syntax. By the description in the issue it seems to me that the goal is to make it possible to load an external file without slowing down starting the application, or at least the perceived time it takes to start it, but in reality it seems to me that that the code will have to wait for download and execution to complete before initialization is done. But I may be missing something here.

If the result is making it possible to use async factory functions, wouldn't it be easier to just declare the factory function async? It seems like the code would handle that as well. Or is the goal to be able to execute async stuff in the side effect of the import?

If this is a pattern that is expected to be fairly common I think it would be nice if it could be configured in an easier to read manner, for example by adding a property that indicates that it should be lazy loaded and only provide the filename. Also what kind of implications does the lazy loading have on how the plugin is written? Will the lazy load work for any existing plugin or will it have to be written in a special way?

[sweco-sedalh]

The primary end goal of this is being able to have a single set of built HTML/JS/CSS that can be configured in various ways (using various controls with different settings, for example using a tool such as Origo Admin which Haninge is working on), such as can already be done today using the built-in controls and #map. However, without lazy loading, this might result in having to load a lot of code that will never be used if using a configuration that does not use the majority of the plugins.

Lazy loading solves this by allowing us to only load the code that is actually used.

A sidenote about loading times

Now, it's very possible to argue that this isn't an issue. Most plugins are only a few tens of kB and most people today are using fast 5G or fiber broadband.

However, one should not simply assume that loading times are always neglible. There are still sufficient parts of Sweden where you can get a slow 4G at best, and especially for a project focused at municipality use we should strive to have a result that is as usable as possible for all citizens.

As a case study, here is Eskilstunakartan loaded using the "Fast 4G" emulation in my browser:

And here "Slow 4G":

There are a lot of "recommendations" about bundle size, but most I've seen are around 100-400 kB, and while those numbers are somewhat useless by themselves they still provide a good indication of what we should aim for.

This code works with any method that lets you load code dynamically via promises, you can implement this yourself (see for example the final example here: https://www.educative.io/answers/how-to-dynamically-load-a-js-file-in-javascript) however modern JavaScript has a dedicated function for this (which is what the example in my OP uses): https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import (this function is also supported by modern bundlers such as Vite).

There sadly is no good way to get a nicer syntax on the user side (the async () => (await import("swiper-plugin")).default), at least that I have found, all attempts break the lazy-loading machinery of the bundler. It should however be noted that using this (lazy loading) is of course entirely optional, and users that can accept non-lazy loading can use the simpler syntax in the LMSearch example.

There are no adjustments needed to plugins, as long as they follow the common pattern of const plugin = Plugin(options); viewer.addComponent(plugin); which most do. It would, in my opinion, make sense to allow creating plugins using new as well (in order to be able to make use of modern JavaScript features), however that is outside the scope of this PR.

I also think that it would make a lot of sense to use lazy loading for some of the built-in components, especially edit (which is somewhat larger yet not used by a lot of maps), which could build on the work done here, but that is outside of the scope.

[steff-o]

I see, I got it mixed up with some sort of deferred load, not associating it with conditional imports (which of course lazy loading is all about).

I see two different use cases for lazy loading here, one is the one you probably is looking for: keep a boilerplate index.html and only have to download the plugins that are activated in config, but also inside a plugin where different parts of the plugin can be lazy loaded. The first case will need some support from Origo, but the second will not.

From what I can read in webpack documentation: https://webpack.js.org/api/module-methods/#dynamic-expressions-in-import webpack has some support for lazy loading. Looks like it will only fully handle dynamic imports when they are inside the same bundling project. A plugin is not, and can therefore not have a chunk created for each plugin. Maybe it is possible to make a general dynamic import statement in origo.js using the magic comment modifiers in order to make it possible to replace the quirky async () => (await import("swiper-plugin")).default statement with just a flag that it should be lazy loaded? If not, it's not a big deal if the intended usage is clearly documented.

Even if it wasn't the intention, this implementation supports async factory functions as the first activation is always awaited, but if it is lazy loaded, async factory functions are not supported as the second invocation is not awaited. Not sure if there is a need for async factory functions or that is a good idea, but awaiting also the second invocation would support it even when lazy loading. Or maybe it shouldn't be allowed at all? Async factory functions could be an antipattern if compared to a constructor that should never fail.

Off topic:

Your idea of lazy loading the editor is interesting as we have been looking for a way to keep the bundle size small for simple public web applications while supporting more advanced GIS functionality (with heavy GIS library dependencies) for internal applications. The way Origo handles that today is expelling that kind of functionality to plugins. The problem is that some types of desired extra functionality can't be written as plugins. For example the editor has no framework for adding new tools, in that case a complete editor plugin must be written to replace the built-in. Being primarily a back end developer, I was looking for some #ifdef counterpart i webpack to make it possible to select whether to include the advanced parts or not, but dynamic import is probably a better way to go as it doesn't require custom builds.

[sweco-sedalh]

Indeed, there are more places where lazy loading could make a lot of sense (as an example, only loading a plugin if it's corresponding button is pressed). That would however be a significantly larger change.

At least with Vite it is also possible the perform lazy loading across package boundaries, and would expect the same to be true with webpack. But for this usage, lazy loading an entire plugin (that is an entire package separately) is enough.

Something we could do to improve the user side syntax is to automatically check if the given control is an object with a .default key, rather than a function, and in that case unpack it in the control initialization code. Then the user code would just be async () => await import("swiper-plugin"), or possibly even () => import("swiper-plugin") (there are a few pitfalls surrounding async-code that might make that second variant not work, but haven't tested this particular case). Should I do that?

I think it can make sense for a factory function to be async, if it has to load some data first for example, but I'm also not opposed to document that as an anti-pattern.

[steff-o]

Something we could do to improve the user side syntax is to automatically check if the given control is an object with a .default key, rather than a function, and in that case unpack it in the control initialization code. Then the user code would just be async () => await import("swiper-plugin"), or possibly even () => import("swiper-plugin") (there are a few pitfalls surrounding async-code that might make that second variant not work, but haven't tested this particular case). Should I do that?

I don't think that it actually makes it easier to configure, it just removes flexibility on how to write the plugin.

I have tested using the Multiselect plugin, which uses the commonly used pattern of exposing the plugin's factory function as a global variable. When I try to lazy load that using you code example pattern but replacing with multiselect multiselect: async () => (await import("/plugins/multiselect.min.js")).default, it does not work.

1. I have to add .js extension
2. The plugin is actually not a module, so there is no .default export, or no export whatsoever

If I change the code to multiselect: async () => { (await import("/plugins/multiselect.min.js")); return document.Multiselect }, and modify the plugin to instead of setting the global var variable creates it as document.Multiselect I can make it work.

I got the impression that it would work with any plugin that:

There are no adjustments needed to plugins, as long as they follow the common pattern of const plugin = Plugin(options); viewer.addComponent(plugin);

but maybe that statement assumed that they are bundled as modules? It's probably hard to make it work for old plugins without adjustments as the global variable will be scoped to the imported module.

Regarding the line:
control.options = { ...control.options, ...controlOptions };
what is the purpose of that? Is it a way to be able to fetch which options were used to create the Control? control.options will be undefined for most controls before the line is executed, I assume that the intention is that the Control itself can set that property to return for instance the effective configuration in use or something, which then is combined with the configured options. But it is not used anywhere?

I played around and tried to make it possible to lazy load using only the path to the plugin as configuration. By replacing

const controlOrLazyLoadedFactory = await controlFactory(controlOptions);
const control = typeof controlOrLazyLoadedFactory === 'function' ? controlOrLazyLoadedFactory(controlOptions) : controlOrLazyLoadedFactory;

with

const control = typeof controlFactory === 'function' ? controlFactory(controlOptions) : (await import(/* webpackIgnore: true */ controlFactory)).default();

I could make it work. But that requires that the plugin is written as a module with the factory function as default export.

[sweco-sedalh]

I believe the discrepancy comes from our use of different bundlers (webpack contra vite), as well as how we depend on the plugin. When using Vite and installing the plugin using an entry in package.json my code example works (it doesn't matter that it itself is not written as a module, vite handles that), as it is indeed provided as a module export: https://github.com/origo-map/multiselect-plugin/blob/main/multiselect.js

Vite or webpack?

As an aside, I think it would make sense to move Origo and its community from webpack to Vite. Vite is considerably faster and easier to configure, and the JavaScript-world as a whole is doing the same switch.

Google Trends: https://trends.google.com/trends/explore?cat=31&date=today%205-y&q=webpack,vite&hl=en-GB
GitHub Stars: https://star-history.com/#Webpack/Webpack&vitejs/vite&Date
NPM Trends (note that this will have a bias towards the incumbent): https://npmtrends.com/vite-vs-webpack

If the community agrees with the change I might be able to do it this this spring using internal funding.

Even if Origo itself is using webpack today I don't think it would be wise to implement functionality that depends on that bundler specifically; other projects wanting to use Origo might want to use a different bundler and that should be an implementation detail. What we could do is allowing to pass a control as a string and load it using the code you provided as well, but that would of course make the code more complex. But with a bit of refactoring it should be managable.

I have not done a deeper analysis of that line, I just rewrote it from control.options = Object.assign(control.options || {}, controlOptions); using newer JavaScript features.

[steff-o]

The multiselect plugin uses webpack configured to bundle the whole shebang as a global var variable even if the code itself is written as a module. I was just testing it out of the box to see what happened when trying to lazy load and tried to figure out what to do when it didn't. It works when not lazy loading, so your code is backwards compatible, which is the most important.

I agree that is not ideal to write the code to be dependent of webpack for bundling. But if other bundlers can handle the import without magic comments it would still work. But since no existing plugins are bundled as modules today they won't work anyway.

I guess we can live with this implementation for now, It will be fairly easy to add support for configuration that is a URL that will automatically be lazy loaded and instantiated if a standard for how to bundle plugins as modules emerges.

I will have another look tomorrow and verify that the additional async stuff does not break anything.

[steff-o]

Well the async stuff seems to check out. I was worried that the async .map() in combination with always awaiting the call to the factory method would mess with the ordering. But as no one uses the return value from the factories until all factories has been resolved it works. Otherwise there could have been a risk that controls that depend on each other would fail.

However, this PR changes the order of the plugins initialization with respect to built in controls. In this PR all controls, plugins and built ins, are created in the order they have been configured in index.json (after the default controls). In the old way of adding plugins all plugins are always created after everything else and if a plugin depends on another control it can safely assume that it exists in onAdd (as order is preserved when adding to viewer).

Not sure how common it is for a plugin to depend on a non-default control and if someone wants to use this new cool way of adding plugins they still need to add the configuration to index.json so they might as well put it in the correct order. In this way it is also more obvious in which order things appear in the menu, so baiscially I think it was a good decision to not have separate section for plugins i index.json.

I will approve this PR, but I will wait to merge it until documentation is present. Please write a PR (no issue needed) in the documentation repo and link it to this PR.