Just check package.json for vendor code up-to-date-ness - #64393
Conversation
typescript-automation
Bot
added
the
For Uncommitted Bug
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation matches the stated simplification and is covered by focused tests.
Review effort: Balanced
Findings: None
What changed in this PR
Simplifies vscode-jsonrpc vendoring freshness checks by tracking only its package manifest.
Changes:
- Uses the vendored
package.jsonas the generation marker. - Updates tests for version-triggered regeneration and
--force.
| File | Description |
|---|---|
Herebyfile.mjs |
Simplifies vendor freshness detection. |
tools/scripts/gen/generatedFile.test.mts |
Tests manifest-based and forced regeneration. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| .map(file => new GeneratedFile(path.join(dest, file), [__filename, path.join(src, file)], undefined, entries)); | ||
| const existingEntries = fs.existsSync(dest) ? fs.globSync("**/*", { cwd: dest }).sort() : []; | ||
| if (JSON.stringify(entries) === JSON.stringify(existingEntries) && generatedFiles.every(file => file.isCurrent(!!options.force))) { | ||
| const manifest = new GeneratedFile(path.join(dest, "package.json"), [__filename, path.join(src, "package.json")]); |
There was a problem hiding this comment.
I don't think this is enough since the version is technically pinned in the lock, right?
There was a problem hiding this comment.
Yeah, but a lockfile update will in turn reinstall a new module with a new package.json, which in turn will allow a new vendor copy.
There was a problem hiding this comment.
Oh, oops, this is the package.json of the actual thing being copied, not the root!
Noticed this while the other PR was merging, this can be way simpler, since it's not like we need to support copying local edits out of node_modules.