Modernize dave to 0.3.0: ESM, promise API, current deps, GitHub Actions - #1
Merged
Conversation
CLAUDE.md captures repo orientation for future sessions; the plan file under .claude/plans/ tracks the modernization tasks for this branch.
istanbul 0.4.5 is unmaintained; its CJS hook fails on current Node because mocha's bundled yargs is now ESM. Swap to c8, which reads V8's native coverage. Test script becomes: c8 mocha 'test/**/*Test.js' && npm run lint Verified: 45 tests pass; coverage report intact; lint clean.
- Add .github/workflows/ci.yml with a Node 22/24 matrix. - Drop .travis.yml. - Swap the build badge in the README. - Emit lcov from c8 so codecov has something to consume.
- Remove .eslintrc.yml; eslint 9 uses the flat config in eslint.config.mjs exclusively and ignores the legacy file. - Pin .nvmrc to Node 24 (current active LTS as of June 2026).
Bring every dep (other than yargs, which is on v13 latest and gets a major bump in its own commit) up to its current major's latest: - async 3.2.5 -> 3.2.6 - chai 4.4.1 -> 4.5.0 - eslint 9.5.0 -> 9.39.4 - js-yaml 4.1.0 -> 4.2.0 (clears the prototype-pollution advisory) - lodash 4.17.21 -> 4.18.1 (clears the unset/omit/template advisories) - mocha 10.4.0 -> 10.8.2
main() and runCommand() now return promises. Internals use async/await
throughout (fs/promises in lib/manifest, a Promise wrapper around yargs
parse callback, util.promisify(exec) for runCommand). The async package
is no longer needed — async.eachLimit(_, 1, _) collapses into a plain
for...of with await.
eslint config caught up to the new code: ecmaVersion bumped to 'latest'
and space-before-function-paren allows the required space between
'async' and '(' on arrow functions.
BREAKING: callback-based callers of main() / runCommand() must switch
to .then() or await. Pre-1.0 so this is an intentional API churn.
- package.json: type: "module". - All source and test files use import/export. - ESM import paths include the .js extension as required. - eslint sourceType bumped to "module". - Bin guard switches from `module === require.main` to comparing process.argv[1] against fileURLToPath(import.meta.url). Reordered before the yargs bump because yargs 18 is ESM-only and can no longer be require()'d. CJS consumers must migrate too — pre-1.0.
API changes: - yargs is now a factory: `yargs(args).command(...).parseAsync()`. - parseAsync() replaces the callback Promise wrapper. - Tags option needs `string: true` because yargs 18 coerces numeric-looking array values to Numbers before custom .coerce() runs (1.0 was collapsing to 1). With `string: true` the original token is preserved and the old .coerce() round-trip is unneeded.
ESM migration in the previous commit unblocks chai 5+ (ESM-only). Existing assert.* usage works unchanged on chai 6.
Lodash was a 70KB dep used in exactly two call sites for _.merge. Inline a 13-line deepMerge in lib/manifest.js with matching semantics (plain-object recursion, source-wins-on-overlap, null source skipped) and drop lodash from dependencies.
npm audit fix took care of flatted, minimatch, and picomatch. Only remaining was serialize-javascript: mocha 10.x pins ^6.0.2 and the advisory ceiling is <=7.0.4, so npm audit fix offered to downgrade mocha to 8.1.3 — unacceptable. Pin via an npm override to ^7.0.5; mocha's HTML reporter doesn't exercise the API surface that changed. npm audit: 0 vulnerabilities.
Adds three small tests: - getMetadata loads a real manifest (happy path was previously untested at this layer; only exercised transitively via main). - getMetadata rejects with ENOENT for a missing path. - getMetadata rejects with YAMLException for malformed YAML. - main propagates ENOENT when the manifest path is bad. test/malformed.yml is a fixture for the YAML failure case. The runCommand failure path is already covered by 'can fail running a command.' on the runCommand suite.
- Bump package.json version to 0.3.0 to reflect the breaking API surface (ESM + promises) and dependency churn on this branch. - README: drop pull/template from the supported-commands list (they were never implemented), trim the stale 0.1.0 sentence, replace the TravisCI example with a GitHub Actions one, and drop the pull/template line items from Future Work. - CLAUDE.md: update internal notes to match the modernized code (deepMerge, yargs 18 + string:true, c8 + lcov, no .eslintrc.yml).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Long-overdue modernization pass on
dave. Twelve focused commits, eachatomic and green, no big-bang changes. Bumps the package to 0.3.0.
Public API — breaking (pre-1.0):
main()andrunCommand()return promises; callback signatures removed."type": "module"). Consumers mustimport(notrequire).Dependencies — all on current majors:
parseAsync,string: trueon the tags option so1.0doesn't collapse to a Number).
async(afor…of+awaitreplaceseachLimit(_, 1, _)).lodash(single use of_.mergereplaced by a 13-linedeepMergewith matching semantics).serialize-javascriptpinned via npmoverridesto clear the onlyaudit finding mocha 10 hasn't picked up.
Tooling / CI:
istanbul→c8(istanbul's CJS hook can't load mocha's now-ESM yargson Node ≥ 22; this was the blocker that started the modernization).
.nvmrc: 22.3.0 → 24..eslintrc.yml; flat config ineslint.config.mjsisthe single source of truth.
Tests:
async/await.getMetadataENOENTand YAMLException;
main()propagation of manifest read errors).lib/yargsandlib/manifest.Audit:
npm auditreports 0 vulnerabilities (down from 12).README: dropped the always-aspirational
pullandtemplatelines;replaced the TravisCI example with a GitHub Actions one.
Test plan
nvm use && npm ci && npm test— 49 passing, lint clean.npm audit— 0 vulnerabilities../index.js build --context . --tags begin end --manifest ./test/manifest.yml— exits 0.