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: pass plugin type when resolving a module path #659

Merged
merged 1 commit into from
Mar 16, 2023

Conversation

cristiand391
Copy link
Member

@cristiand391 cristiand391 commented Mar 15, 2023

This fixes a bug where linked plugins with hooks were causing a CLI to fail with the ModuleLoadError: [MODULE_NOT_FOUND] require failed to load.

#650 introduced compilation at runtime for linked TS plugins using ts-node.
When trying to compile hooks from linked plugins oclif isn't passing the plugin.type prop to the ts path resolver so tsPath was just skipping the compilation step.

Before:

Screenshot 2023-03-16 at 10 07 02

After

Screenshot 2023-03-16 at 10 07 32

Testing

Repro

  • cd to plugin-info (doctor hook)
    • run rm -rf lib
    • run ../sfdx-cli/bin/run plugins:link .
    • run ../sfdx-cli/bin/run, should see the MODULE_NOT_FOUND

QA

  • yarn link @oclif/core in sfdx-cli
  • cd to plugin-info (doctor hook)
  • run rm -rf lib
  • run ../sfdx-cli/bin/run plugins:link .
  • run ../sfdx-cli/bin/run, CLI should work.

try {
filePath = require.resolve(modulePath)
isESM = ModuleLoader.isPathModule(filePath)
} catch {
filePath = Config.tsPath(config.root, modulePath)
filePath = isPlugin(config) ? Config.tsPath(config.root, modulePath, config.type) : Config.tsPath(config.root, modulePath)
Copy link
Member Author

@cristiand391 cristiand391 Mar 15, 2023

Choose a reason for hiding this comment

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

pass plugin.type to Config.tsPath if config is a plugin.

hooks get loaded here:
https://github.com/oclif/core/blob/main/src/config/config.ts#L310

usually these are paths to compiled JS files like this one:
https://github.com/salesforcecli/plugin-info/blob/main/package.json#L93

Linked plugins are supposed to get compiled at runtime by ts-node but hooks were being skipped because the plugin.type wasn't being passed to tsPath:
https://github.com/oclif/core/blob/main/src/config/ts-node.ts#L101

commands from linked plugins are loaded here (see plugin.type is being passed):
https://github.com/oclif/core/blob/main/src/config/plugin.ts#L182

@@ -136,11 +136,15 @@ export default class ModuleLoader {
let isESM: boolean
let filePath: string

const isPlugin = (config: IConfig|IPlugin): config is IPlugin => {
Copy link
Member Author

Choose a reason for hiding this comment

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

TS guard, used to pass plugin.type when config comes from a plugin.

@mshanemc mshanemc added the bug Something isn't working label Mar 16, 2023
@git2gus
Copy link

git2gus bot commented Mar 16, 2023

This issue has been linked to a new work item: W-12702335

@mshanemc
Copy link
Member

QA: followed steps above setup steps above.

Modify linked plugin (doctor) and reran, change runs from TS with no compilation required.

Screenshot 2023-03-16 at 8 39 26 AM

modified the hook, change also reflected immediately
Screenshot 2023-03-16 at 8 42 56 AM

@mshanemc mshanemc merged commit d5f58a3 into main Mar 16, 2023
@mshanemc mshanemc deleted the cd/fix-ts-node-hooks branch March 16, 2023 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants