Add generic middleware support

[andersevenrud]

It would be nice to have a way to add middleware support to packages.

An example of use-case is to have dynamic context menus in file manager based defined by other packages.

Ref: https://github.com/os-js/OS.js/issues/752

• [x] Add middleware provider
• [x] Add package middleware script loading strategy

[andersevenrud]

From Gitter discussion:

---

@MoradiDavijani

Can you give me a hint about how should I handle middleware.js in osjs-client? I don't know what exactly happens when loading applications 😅

@andersevenrud

Since all of the core service contracts are registered before the package manager initializes, I think Packages#addPacakges would be a nice fit for this.

It can be converted to an async function that filters out the added list with onces that have a "background" script defined in them, then proceeds to load them.

This means that the regular package launch needs to filter out these "background" scripts as well.

[ThisIsOstad]

Do you mean defining the background scripts like this in packages metadata?

// Package metadata
{
  "background": [
    "./middleware.js"
  ]
}

The addPackages will check for background property and loads the files, right?

This means that the regular package launch needs to filter out these "background" scripts as well.

I'm not sure I've understood it completely, could you elaborate on it?

[andersevenrud]

Do you mean defining the background scripts like this in packages metadata?

Correct.

The addPackages will check for background property and loads the files, right?

Yup.

I'm not sure I've understood it completely, could you elaborate on it?

The "background" scripts in metadata.json should only be loaded on Packages#addPackages.

Right now when you launch a package, all scripts, which will include the "background" ones.

[andersevenrud]

Do you mean defining the background scripts like this in packages metadata?

I actually thought to do it this way:

{
  "preload": [
    {
      "background": true,
      "filename": "middleware.js"
    }
  ]
}

But maybe having a separate property as you described would be better for this :thinking:

[ThisIsOstad]

Do we already have a preload property in metadata.json files?

[andersevenrud]

My mind is a bit fuzzy from what I thought in https://github.com/os-js/OS.js/issues/752, but I probably meant files here, not preload :blush: That property already exists and is what is used in Packages#_launch.

[ThisIsOstad]

It can be converted to an async function ...

Is it compatible with other packages? As the addPackages is exposed and can be used by other packages, maybe it's better to keep it sync, and load background scripts without blocking this function? 🤔

[ThisIsOstad]

And how about adding a preload property as an array of strings (like files)?

[andersevenrud]

Is it compatible with other packages? As the addPackages is exposed and can be used by other packages, maybe it's better to keep it sync, and load background scripts without blocking this function? thinking

I'm not actually sure that this is used anywhere, but you're right.

Probably better to leave that as-is and then in the Packages#init chain add another call to a method, ex Packages#_preloadBackgroundFiles that takes the current package list and loads the scripts.

A down-side with this would be that doing addPackages from the service provider on runtime would not pre-load these files.

However, this can by making it so that the addPackages method adds a special key to the metadata (only if init() has been run) that tells _launch to load the "background" file.

[andersevenrud]

And how about adding a preload property as an array of strings (like files)?

Yeah, I'm not really sure about adding a new property.

The files array could be automatically expanded, ex:

files: [
  'a',
  'b',
  {filename: 'c'},
  {filename: 'd', background: true}
]

becomes

files: [
  {filename: 'a', background: false},
  {filename: 'b', background: false},
  {filename: 'c', background: false},
  {filename: 'd', background: true}
]

By doing doing this it generalizes it, making it possible to fit in more types of "file" scenarios in the future :thinking:

[andersevenrud]

By doing doing this it generalizes it, making it possible to fit in more types of "file" scenarios in the future thinking

In this case, having type: 'background' might be better and type: 'preload' is default.

[ThisIsOstad]

What happens if we load background files in addPackages but do not wait for them?

addPackages(list) {
 this._preloadBackgroundFiles(list); // its an async method but we don't wait for it
 ...
 return ...
}

[andersevenrud]

What happens if we load background files in addPackages but do not wait for them?

A side-effect like this could cause applications to launch before middleware was registered properly. Once the package manager has inited the session will be restored.

[ThisIsOstad]

Is it compatible with other packages? As the addPackages is exposed and can be used by other packages, maybe it's better to keep it sync, and load background scripts without blocking this function? thinking

I'm not actually sure that this is used anywhere, but you're right.

Probably better to leave that as-is and then in the Packages#init chain add another call to a method, ex Packages#_preloadBackgroundFiles that takes the current package list and loads the scripts.

A down-side with this would be that doing addPackages from the service provider on runtime would not pre-load these files.

However, this can by making it so that the addPackages method adds a special key to the metadata (only if init() has been run) that tells _launch to load the "background" file.

Now I think it's better to make addPackages an async function and fix packages that are using it (if any) 😁 What do you think?

[andersevenrud]

What do you think?

I've looked through the official packages, and this is not used anywhere. However, I don't know if anyone else is actually using this :thinking:

I think a good compromise (at least for now) is just to don't allow the background files for dynamically added packages, and just update the init() method :smile:

[andersevenrud]

and just update the init() method

Well, the Packages#addPackages must also be changed to expand the files array, and then when the Preloader#load is called, the given list must be collapsed into string[] ( .map(file => file.filename)).

So, in total:

1. Add a new method to Packages that preloads the background scripts on init()
2. Change the addPackages to expand files array
3. Change _launch to collapse and filter the background files for the preloader

I think maybe the background script loading errors should be discarded and just emit a error log as to not stop the init process.

[andersevenrud]

I just released @osjs/client@3.5.0 with these changes!

Thank you very much for your work :smile:

[andersevenrud]

I've opened an issue about documentation of these features here: https://github.com/os-js/manual.os-js.org/issues/33