Readme Command Runs Active Project's Init Hooks.

rfrazierADI commented 1 year ago

Do you want to request a feature or report a bug?

bug

What is the current behavior?

running oclif readme for a project runs that project's init hooks.

If the current behavior is a bug, please provide the steps to reproduce.

use oclif to generate a new cli
add an init hook and register it with the package.json
run oclif readme in the project directory
One of two things will happen: if the project has been built, the init hook runs, otherwise it will raise a module load error:
```
ModuleLoadError: [MODULE_NOT_FOUND] require failed to load /Users/Ryan.Frazier/.../init/...: Cannot find module 
```

What is the expected behavior?

Running the readme command should not run hooks that belong to the CLI being developed. From looking at the code this seems intentional but I cannot understand why I would want this behaviour. https://github.com/oclif/oclif/blob/9770d1dba2e0cd66fc99837890b64501f97d9a98/src/commands/readme.ts#L54

oclif/3.17.2 darwin-x64 node-v19.8.1

mdonnalley commented 1 year ago

@rfrazierADI I couldn't reproduce the issue with these steps

oclif generate gh-1208-cjs // selected CJS

cd gh-1208-cjs
oclif generate hook --event init initHook

oclif readme // works

rm -rf dist
oclif readme //works

oclif generate gh-1208-esm // selected ESM

cd gh-1208-esm
oclif generate hook --event init initHook

oclif readme // works

rm -rf dist
oclif readme //works

Can you create a repo that replicates the issue? Thanks!

rfrazier716 commented 1 year ago

Here's a repository that replicates the bug: https://github.com/rfrazier716/oclif-init-bug

steps:

clone repository
cd into project
yarn install
oclif readme (generates below output)

> oclif readme
example hook running readme
replacing <!-- usage --> in README.md
replacing <!-- commands --> in README.md
replacing <!-- toc --> in README.md

The line example hook running readme is the one generated by a hook in the project repository. Also, I put the wrong version in the initial bug report. Actual version is:

> oclif --version
oclif/4.0.3 darwin-x64 node-v18.17.1

mdonnalley commented 1 year ago

@rfrazier716 I'm not seeing your original ModuleLoadError: [MODULE_NOT_FOUND] error following those steps. Is that the issue you're wanting to solved? Or you just don't think it should run that init hook at all in this scenario?

rfrazier716 commented 1 year ago

Looks like the ModuleLoadError is from a version mismatch between the version of oclif in yarn.lock (3.17.2) and my global installation (4.0.3), and the error was fixed somewhere between those versions.

4.0.3 behaviour:

> rm -rf dist && oclif readme
 ›   Warning: No compiled source found at dist. Some commands may be missing.
replacing <!-- usage --> in README.md
replacing <!-- commands --> in README.md
replacing <!-- toc --> in README.md

3.17.2 behaviour:

> rm -rf dist && yarn run oclif readme
yarn run v1.22.19
$ /Users/.../oclif-init-bug/node_modules/.bin/oclif readme
 ›   Warning: No compiled source found at dist. Some commands may be missing.
 ›   ModuleLoadError: [MODULE_NOT_FOUND] require failed to load /Users/.../oclif-init-bug/dist/hooks/init/initHook: Cannot find module

The bug I'd like solved/to understand more is why the readme command is running my CLI's init hooks. We use those to e.g. alert the user they're testing a beta version or check if it's the first time the cli is run. I don't know why those should need to run during readme generation so I'd love to understand if (1) the init hooks being run is a bug, or (2) if it's not a bug, am I not using the init hooks as intended?

mdonnalley commented 1 year ago

It's not a bug, that runHook line has been in the code since the command was first released in 2018: https://github.com/oclif/dev-cli/commit/fa21751482e682896b1c3c92fcd035d3ca80d61e

I wasn't around at the time but I have a couple of guesses why it might be there:

It might need to run init so that if your plugin has @oclif/plugin-legacy installed, it can run that plugin's init hook (@oclif/plugin-legacy exists to migrate cli-engine (oclif predecessor) to oclif at run time)
It might be supporting plugins that dynamically load commands with their init hook

To answer your second question: you are using init hooks correctly. If you really need to prevent those from running during readme, the command id is available inside of the hook so you could exit early if it's the readme command, e.g.

if (opts.command === 'readme') return

rfrazier716 commented 1 year ago

Thanks for the clarification @mdonnalley. Will use the early exit method going forward if we don't want things run during the readme.

It might be supporting plugins that dynamically load commands with their init hook

Did not know that was possible, very cool!

mdonnalley commented 1 year ago

Did not know that was possible, very cool!

It's not officially supported but there is a plugin out there that supposedly does it: https://github.com/DanHulton/oclif-dynamic-commands

oclif / oclif