adapt-security / adapt-authoring

A server-based user interface for authoring eLearning courses using the Adapt framework.
http://adaptlearning.org
10 stars 5 forks source link

Defining interdependencies between Adapt modules #629

Closed taylortom closed 4 weeks ago

taylortom commented 8 months ago

We need to establish the best way to define dependencies between Adapt modules*

Adapt modules fall into two categories:

  1. 'Hard' dependencies - where a module needs to use a specific export from another Adapt module (e.g. if extending from another class such as AbstractModule). In this case, a missing module will likely cause an error during app initialisation.
  2. 'Loose' dependencies - where a module imports another module using the AAT's internal dependency manager (i.e. waitForModule). In this case, a missing module will likely cause an unexpected runtime error at some point in the future.

We have the following requirements of Adapt module dependency management:

We currently handle all interdependencies between Adapt modules with NPM, using a mix of NPM's dependencies, peerDependencies, and a package-lock.json file.

On peerDependencies (from the NPM docs):

In some cases, you want to express the compatibility of your package with a host tool or library, while not necessarily doing a require of this host. This is usually referred to as a plugin. Notably, your module may be exposing a specific interface, expected and specified by the host documentation.

Whether this is the most sensible approach remains to be seen. This issue has been created for discussion purposes.

See the below issues for some related discussion:

Versioning

Following the above, a solution for versioning a current state of the app/dependencies needs to be agreed upon and documented. We want simple (minimal effort required from devs) and robust (version clamping within a single AAT release).

As with the framework, we should introduce automation via GitHub actions where possible. We need to agree on how releases are triggered (automatic or manual) - It's likely to be a common occurrence that a feature requires changes in multiple modules, so an automatic release mechanism for adapt-authoring may not be possible (although this could/should be implemented for the Adapt modules).

Use cases

See below for some common example use cases for a dependency management mechanism:

Please comment below with thoughts/opinions.

* Where an 'Adapt module' here refers to the AAT-specific type of Node modules which form the building blocks of the AAT v1.

chris-steele commented 7 months ago

I'll try to explain by way of example my observations/concerns with the way we handle dependencies in the AAT.

adaptframework specifies content as a dependency. If I am working on a dev branch of content in my local workspace then when Node comes to resolve the content dependency for adaptframework it will install a copy of the master branch of content locally in adaptframework/node_modules.

Now, given that Node links workspace folders into the root node_modules folder it will be the dev branch of content that gets loaded via the AAT's internal dependency loader. And given that code requiring content does so via this mechanism then the expected behaviours are obtained. However, if code was imported via ESM then problems would likely occur...

I noticed certain base packages like api/core were specified as dependencies in several packages. If dev work was to be done on these base packages then we'd likely find that packages load master branch code rather than the expected workspace code. Consequently I've amended packages specifying these as dependencies by making the base packages peerDependencies.

This still leaves the problem of AAT packages that specify other AAT packages as dependencies: things may work because dependencies are resolved via the AAT runtime dependency loader, but it is somewhat by fluke and at best it is clumsy to have multiple copies of packages installed and unused.

chris-steele commented 7 months ago

Update following discussion with @taylortom and @oliverfoster: