feat: add replacements skill#1
Conversation
mcmxcdev
left a comment
There was a problem hiding this comment.
Feels like it's in good enough shape to ship as a first POC that can be iterated on!
| - **`documented`** — swap to another package (`replacementModule`); a leaner, | ||
| better-maintained equivalent. | ||
| - **`native`** — drop the package for a built-in platform feature (`description` | ||
| explains which; `nodeFeatureId`/`webFeatureId` identify it). Mind `engines`. | ||
| - **`simple`** — replace usage with a short inline snippet (`description`, and | ||
| `example` code when present); typically a micro-utility you don't need a | ||
| dependency for. | ||
| - **`removal`** — the package can be removed outright (`description` says why). |
There was a problem hiding this comment.
For lodash, there were multiple checkboxes like "lodash > es-toolkit" AND "lodash -> native JS".
I was able to select both, but in the next step AI noticed that they're mutually exclusive and asked to clarify further with "hybrid", "all es-toolkit" and "all native js" options.
Its a matter of taste if thats up to the user to work through the options correctly I guess.
There was a problem hiding this comment.
Could benefit from some test coverage
There was a problem hiding this comment.
I tested this skill on a monorepo, and felt like I would rather use CLI/codemod that might be slightly wrong but provide instant feedback. AI consumed a lot of tokens and took many minutes to get something done. The changes generally were pretty clean, with some replacements that would have needed follow ups.
| } | ||
|
|
||
| async function fromPackageJson(): Promise<string[]> { | ||
| const pkg = JSON.parse(await readFile('package.json', 'utf8')); |
There was a problem hiding this comment.
Running match.ts against the root of a monorepo doesn't detect e.g. lodash in nested package.json, only at root level. Intentional?
| 2. Run the matcher with the collected names: | ||
|
|
||
| ```sh | ||
| node <skill-dir>/match.ts <name> <name> ... |
There was a problem hiding this comment.
Why not just tell the agent to run ./match.js? So you don't have to specify what is <skill-dir> (it adds tokens, and it's more confusing...the agents are smart enough to figure out how to run ./match.js
|
|
||
| Present the matches grouped by current module. For each, state the suggested | ||
| replacement(s), the type/rationale in a few words, any engine caveats, and the | ||
| `docUrl`. When several replacements are offered, lead with the `preferred` one. |
There was a problem hiding this comment.
Should we also say to mark the preferred one as preferred? Also I wonder if we could add a reason for it to be preferred in the module replacement data (but we can also tell the agent to try and figure out why is preferred...this is a bit of a stretch tho)
| `vendor/` holds the `module-replacements` package copied verbatim (its compiled | ||
| `dist` and `manifests`), produced by `scripts/vendor-replacements.ts` at the repo | ||
| root (run `npm run build:data`). Do not edit anything under `vendor/` by hand; | ||
| re-run the script after bumping the `module-replacements` devDependency. |
There was a problem hiding this comment.
Mentioning scripts/vendor-replacements.ts is detrimental here IMHO...it will not be included in the SKILL when it's installed with something like skills.sh, and it's referencing the root repo.
However, I wonder if we should add the script to vendor it inside the SKILL so that the AI (or the user) can also update the package if a new version comes out.
This basically vendors the module-replacements data set and adds an associated skill.
The reason for the vendoring is so the agent doesn't have to hit an external tool to query. Otherwise, it would have to install the CLI, or install module-replacements.
it may be worth seeing how the modern-web-guidance does this with their data set in case we can do similar.