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

Update all modules' peerDependencies #493

Open taylortom opened 2 years ago

taylortom commented 2 years ago

Need to make sure all module interdependencies are defined using the peerDependencies object.

This includes the following:

See this article for a bit of an explanation.

chris-steele commented 8 months ago

@taylortom I've taken down the comments I added to make some revisions - will post back soon.

chris-steele commented 8 months ago

UPDATE on the below comments: at present I am correcting dependencies across packages and adding peerDependencies as per OP to verify Node behaviour (especially w.r.t. workspaces).

Peer dependencies are for plugin architectures. The AAT doesn't implement a plugin architecture: it is a single application split across packages. These packages have a mix of import and runtime interdependencies. Most packages are therefore coupled and some tightly (jsonschema <-> config, content <-> contentplugin).

Propose the following:

  1. drop the notion of peer dependencies completely
  2. remove all adapt-* dependencies from package.json files and hoist them to the root project package.json (adapt-authoring)

This will mean we have a single place to change paths (e.g. select a different Git branch) for whatever dependencies we like and therefore not have resolution conflicts when we are using Node workspaces.

For example, if I am working on adaptframework issue/31, and content issue/23 (but from a different Git user) I can just edit one package.json with:

"adapt-authoring-adaptframework": "github:adapt-security/adapt-authoring-adaptframework#issue/31", "adapt-authoring-content": "github:chris-steele/adapt-authoring-content#issue/23"

taylortom commented 3 months ago

As discussed with Ollie, we'll keep nested dependencies to allow for simpler installations and a more transparent dependency map.

After running a few tests on peer dependencies, I agree that they introduce more problems than they solve, so think we need to forget that approach for now. I do still think we need a way for developers to flag when certain modules are used at runtime to give useful warnings to the user.