Plugin Repo: Partially resolves #9280: Allow marking specific NPM packages as superseded

[personalizedrefrigerator]

Summary

This pull request allows changing the NPM package associated with a plugin by creating a list of ignored NPM packages.

A plugin can now be overridden by

1. adding a package to the set of ignored NPM packages,
   • this is done by adding the _superseded_package key to manifestOverrides.json. The _superseded_package is ignored.
2. updating the _npm_package_name value in manifests.json to the new NPM package, and
3. re-running the plugin update script.

Notes

A note on the approach taken by this pull request

My original approach involved adding a map from original package names to updated package names. However, this approach would involve adding a special case for plugin overrides to checkIfPluginCanBeAdded.ts , while this approach does not.

Additionally, given the check that verifies that previous versions of a plugin have the same name as the latest, the new _superseded_package key shouldn't be necessary, if _npm_package_name is updated in manifests.json. However, this change should provide a more informative message when a plugin is moved and creates an easily-accessible list of all moved packages.

A note on overriding the NPM package name for the first time

A follow-up pull request should document this process in a file in the plugin repository's readme directory.

With this pull request, it involves updating manifests.json manually once and adding the package name to manifestOverrides.json. For example,

// in manifestOverrides.json
...
    "joplin.plugin.benji.favorites": {
        "_superseded_package": "joplin-plugin-benji-favorites"
    },
...

Testing

Testing done so far

• Verified that a local copy of the plugin repository can be built and prints that a specific NPM package is ignored.

Testing to do

• Create a new plugin with the same ID (but different NPM package from) an existing plugin. Verify that updating _npm_package_name in manifests.json allows updating the plugin from the new NPM package.
  • I'm currently hesitant to do this as it involves publishing a new plugin to NPM just for testing purposes. Would it make sense to publish an override package for the favorites plugin now and test with that package?
  • Edit: I published this plugin, which has the same ID as the original favorites plugin.
• Connect a client to the test plugin repository and verify that the overridden plugin is used.
  • Note: This should be doable by uncommenting and modifying this line (and commenting out the line above).

[laurent22]

Additionally, given the check that verifies that previous versions of a plugin have the same name as the latest, the new ignoredPackages.json shouldn't be necessary, if _npm_package_name is updated in manifests.json. However, this change should provide a more informative message when a plugin is moved and creates an easily-accessible list of all moved packages.

It would be good if we didn't need this extra ignoredPackages.json although I understand why you've added it.

When changing _npm_package_name how about we also add to manifestOverride.json a key such "_superseded_package": "previous-package-name". That way we keep this record to find out what happened and we don't need an extra file. Do you think that would work?

[tessus]

I'm currently hesitant to do this as it involves publishing a new plugin to NPM just for testing purposes. Would it make sense to publish an override package for the favorites plugin now and test with that package?

You don't have to. The following package is still tried to be added to the plugin list, even though it is in the manifestOverrides.json (entry joplin-001):

> npm install joplin-plugin-rohan --save --ignore-scripts
Error: ID cannot be shorter than 16 characters
    at default_1 (/home/joplinbot/.npm-global/lib/node_modules/@joplin/plugin-repo-cli/node_modules/@joplin/lib/services/plugins/utils/validatePluginId.ts:4:80)
    at /home/joplinbot/.npm-global/lib/node_modules/@joplin/plugin-repo-cli/index.ts:71:18
    at Generator.next (<anonymous>)
    at fulfilled (/home/joplinbot/.npm-global/lib/node_modules/@joplin/plugin-repo-cli/index.js:7:58)

[personalizedrefrigerator]

I've tested this in a Windows VM by:

1. Resetting Joplin (git bash: rm -rf ~/.config/joplindev-desktop)
2. Cloning the plugin repository (cd ~/Documents; git clone https://github.com/joplin/plugins.git)
3. Removing the repository's remote URL (git remote set-url origin '')
4. Configuring Joplin to use the custom plugin repository by replacing this line with

	repoApi_ = new RepositoryApi('/Users/User/Documents/plugins', Setting.value('tempDir'));

and commenting out the previous line.

5. Building and starting the desktop application (yarn tsc && yarn start)
6. Installing the favorites plugin
7. Restarting the app
8. Adding a few favorites
9. Closing Joplin and opening the cloned plugin repository
10. Adding

    "joplin.plugin.benji.favorites": {
        "_superseded_package": "joplin-plugin-benji-favorites"
    }

to the end of manifestOverrides.json in ~/Documents/plugins/manifestOverrides.json
11. Replacing "_npm_package_name": "joplin-plugin-benji-favorites", with "_npm_package_name": "joplin-plugin-personalizedrefrigerator-favorites",
12. Committing
13. Building and running the plugin repo cli script for the new repository (yarn start build ~/Documents/plugins/)
14. Stopping early with ctrl-c (running the script can take a long time).
15. Adding the word favorite to the search query in index.ts to only fetch relevant packages. Diff:

diff --git a/packages/plugin-repo-cli/index.ts b/packages/plugin-repo-cli/index.ts
index 65e8b281c..3b512468e 100644
--- a/packages/plugin-repo-cli/index.ts
+++ b/packages/plugin-repo-cli/index.ts
@@ -241,7 +241,7 @@ async function commandBuild(args: CommandBuildArgs) {

        chdir(previousDir);

-       const searchResults = (await execCommand('npm search joplin-plugin --searchlimit 5000 --json', { showStdout: false, showStderr: false })).trim();
+       const searchResults = (await execCommand('npm search "joplin-plugin favorite" --searchlimit 5000 --json', { showStdout: false, showStderr: false })).trim();
        const npmPackages = pluginInfoFromSearchResults(JSON.parse(searchResults));

        for (const npmPackage of npmPackages) {

16. Rebuilding and re-running (yarn tsc && yarn start build ~/Documents/plugins/)
17. Starting the desktop client again
18. Updating the plugin
19. Restarting
20. Verifying that the original favorites are still present and the click issue has been fixed

I think this is ready for review/merge! Related (proof of concept) pull request.

[tessus]

btw, any way you can find out why the "_obsolete": true is ignored for the plugin joplin-001? Hmm, maybe because the _npm_package_name is missing?

[personalizedrefrigerator]

btw, any way you can find out why the "_obsolete": true is ignored for the plugin joplin-001? Hmm, maybe because the _npm_package_name is missing?

Currently extractPluginFilesFromPackage is called before checking if (!obsoleteManifests[manifest.id]) {:

joplin/packages/plugin-repo-cli/index.ts

Lines 168 to 174 in 1af46b0

	manifest = await extractPluginFilesFromPackage(existingManifests, packageTempDir, npmPackage.name, destDir);
	if (!existingManifests[manifest.id]) {
	actionType = ProcessingActionType.Add;
	}
	if (!obsoleteManifests[manifest.id]) {

The Error related to joplin-001 should be thrown while processing the package, and thus before _obsolete is taken into account.

[tessus]

Ah, I see. Thanks a bunch.

[laurent22]

Looks like it's ready to merge. @personalizedrefrigerator, if you could also add a readme on the plugin repo to document this process that would be great.

[laurent22]

The Error related to joplin-001 should be thrown while processing the package, and thus before _obsolete is taken into account.

It would be good to clear this warning actually. Maybe we could have a check - if the plugin is marked as obsolete, we don't print an error related to the invalid plugin ID?