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

Fix: Clear up workspace subdependencies (fixes #629) #630

Closed oliverfoster closed 1 month ago

oliverfoster commented 7 months ago

fixes #629

Allows workspaces to use giturl dependencies and peer dependencies by cleaning up erroneously installed duplicates of the workspace modules.

Fix

Test

git clone https://github.com/adapt-security/adapt-authoring 629-workspacesubdependencies
cd 629-workspacesubdependencies
git checkout issue/629
git clone https://github.com/adapt-security/adapt-authoring-contentplugin local_adapt_modules/adapt-authoring-contentplugin
git clone https://github.com/adapt-security/adapt-authoring-core local_adapt_modules/adapt-authoring-core
npm install --force # to override peer dependency resolve errors

image

chris-steele commented 7 months ago

First pass was excellent @oliverfoster :

Post intall script removed erroneous sub deps in workspaces and also in root modules (coursetheme and mongodblogger)

Ran node ./bin/start.js - tool built and started with no issues 💯

oliverfoster commented 7 months ago

This will all hopefully be fixed by itself for the most part once the repos are registered on npm. This is as the subdependencies when defined with "name": "version" should resolve against workspace modules properly where they don't with "name": "giturl".

chris-steele commented 7 months ago

@oliverfoster is it possible to have this run after npm update? I just realised after running that command all the erroneous installs return.

oliverfoster commented 7 months ago

Probably. Although it may be that you just need to run npm install to do updates.

Here's the documentation for how scripts are named with pre and post.

https://docs.npmjs.com/cli/v10/using-npm/scripts

And the update command.

https://docs.npmjs.com/cli/v10/commands/npm-update

You could also run it manually with npm run postinstall

chris-steele commented 7 months ago

Great tips thank you 💯

oliverfoster commented 7 months ago

@chris-steele did you find a way to get it to work on npm update?

chris-steele commented 6 months ago

Yes @oliverfoster I can run npm run postinstall after npm update and it works as expected 👍

oliverfoster commented 6 months ago

Did you try adding the same script at postupdate?

oliverfoster commented 4 months ago

Is there any progress on this?

chris-steele commented 1 month ago

I did try postupdate, but I don't think it is available with npm

oliverfoster commented 1 month ago

It is not, correct.

https://github.com/npm/rfcs/discussions/226

chris-steele commented 1 month ago

Looks to have fizzled out and died a death