Add plugin for moonrepo (moonrepo.dev) #579
Conversation
| assert.deepEqual(counters, { | ||
| ...baseCounters, | ||
| files: 2, | ||
| devDependencies: 4, |
There was a problem hiding this comment.
Any chance you could add assertions about issues.devDependencies for those 4?
|
|
||
| const isEnabled: IsPluginEnabled = ({ dependencies }) => hasDependency(dependencies, enablers); | ||
|
|
||
| const config: string[] = ['**/moon.yml', '.moon/tasks.yml', '.moon/tasks/*.yml']; |
There was a problem hiding this comment.
Whenever possible I try to prevent globs starting with ** (as they tend to be more "expensive"). Can we do without, or is there maybe a default location for projects so we can use that default here as well?
There was a problem hiding this comment.
I didn't know how to test this where there's various "workspaces", as I'm trying to simulate a monorepo. So the .moon/tasks.yml and .moon/tasks/*.yml are going to be against the workspace root, while moon.yml lives in the project (package) root. How would I test that?
There was a problem hiding this comment.
nvm figured it out
| assert(issues.binaries['.moon/tasks/typescript.yml']['eslint']); | ||
| assert(issues.binaries['apps/a/moon.yml']['vite-node']); | ||
|
|
||
| assert(issues.files.has(join(cwd, 'tools/linters/lint-readme.ts'))); |
There was a problem hiding this comment.
vite-node $workspaceRoot/tools/linters/lint-readme.ts
Ideally, this file would be used, but the plugin doesn't replace $workspaceRoot (and $projectRoot. As the plugins receive the YAML contents (as a JS object) I think we can do this replacement before handing it over to getDependenciesFromScripts? However, I realize that plugins have options.config.cwd, but not the path to the "repo root" (basically process.cwd()). For now we can use process.cwd() and I can fix that up later on by adding it to those options.
Is this something you could work into this PR? Otherwise I'm happy to take a stab at it.
There was a problem hiding this comment.
Ya I understand what you're going for. Let me see what I can do.
There was a problem hiding this comment.
The solution looks correct, but I'm still seeing the file unused? What I usually do to debug is something like npx tsx ../../src/cli.ts --debug from inside the fixtures folder, that should show some insights into what the plugin returns (again, I'm happy to take over, but of course it's cool if you could wrap this up).
There was a problem hiding this comment.
So I played around with it a bunch and I'm kind of stumped. Is there something funky going on with the runtime?
When I logged the command on this line, it's correct. When I go into getDependenciesFromScripts and log npmScripts on literally the first line, it's showing an empty array.
Any idea what's going on there?
There was a problem hiding this comment.
- The tests are "end-to-end" so the first invocation is likely from
package.json. - I saw a few things were not in place at all, so I just went ahead and added those in this PR (and will clean up later after merging)
|
Think I addressed everything! Let me know if there's anything else. |
|
If you're OK with this, I'll merge it! |
|
Odd, those commits after rebase, no worries about that btw. |
|
Yup, merge away! Thanks. |
|
🚀 This pull request is included in v5.9.0. See Release 5.9.0 for release notes. Using Knip in a commercial project? Please consider sponsoring me. |
No description provided.