Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle legacy plugins #582

Merged
merged 10 commits into from
Apr 4, 2023
Merged

Conversation

cristiand391
Copy link
Member

@cristiand391 cristiand391 commented Apr 3, 2023

Fix plugins:install failing to install legacy (cli-engine, old heroku) plugins.

After moving to latest oclif/core and plugin-plugins, the heroku CLI started to fail when trying to install legacy plugins, probably caused by this code:
https://github.com/oclif/core/blob/a18803454449986e03549838ba3928c66f98da37/src/config/plugin.ts#L161-L165

heroku has plugin-legacy as a dependency to make legacy plugins work with latest oclif/core:
https://github.com/heroku/cli/blob/master/packages/cli/package.json#L39

init hook that does the plugin conversion at runtime:
https://github.com/oclif/plugin-legacy/blob/main/src/hooks/init.ts

This PR fixes the issue by checking if the plugin being installed was loaded by plugin-legacy as an instance of PluginLegacy.

@@ -366,4 +348,22 @@ export default class Plugins {
)
return plugins
}

private isValidPlugin(p: Config): boolean {
if (p.valid) return true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is set here to differentiate between oclif/core and cli-engine plugins:
https://github.com/oclif/core/blob/a18803454449986e03549838ba3928c66f98da37/src/config/plugin.ts#L161-L165

if (!this.config.plugins.find(p => p.name === '@oclif/plugin-legacy') ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore check if this plugin was loaded by `plugin-legacy`
p._base.includes('@oclif/plugin-legacy')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we need to check if the plugin being linked/installed was loaded using the PluginLegacy class in the plugin-legacy init hook (which should be included in the CLI, not the plugin):
https://github.com/oclif/plugin-legacy/blob/main/src/hooks/init.ts#L11

This should be done using instanceof PluginLegacy but we would have to include plugin-legacy as a dependency of plugin-plugins, so instead just check if p._base includes @oclif/plugin-legacy.

_base is set in oclif/core when creating an instance of Plugin here:
https://github.com/oclif/core/blob/a18803454449986e03549838ba3928c66f98da37/src/config/plugin.ts#L98

but plugins loaded by plugin-legacy hook will have it set to @oclif/plugin-legacy@<version>:
https://github.com/oclif/plugin-legacy/blob/main/src/index.ts#L44

@cristiand391 cristiand391 merged commit 3c50914 into main Apr 4, 2023
@cristiand391 cristiand391 deleted the prerelease/fix-install-legacy-plugins branch April 4, 2023 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants