-
Notifications
You must be signed in to change notification settings - Fork 22
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
Conversation
@@ -366,4 +348,22 @@ export default class Plugins { | |||
) | |||
return plugins | |||
} | |||
|
|||
private isValidPlugin(p: Config): boolean { | |||
if (p.valid) return true |
There was a problem hiding this comment.
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') |
There was a problem hiding this comment.
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
Fix
plugins:install
failing to install legacy (cli-engine
, old heroku) plugins.After moving to latest
oclif/core
andplugin-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
hasplugin-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 ofPluginLegacy
.