refactor to get rid of legacy libraries and legacy js patterns#50
Merged
Conversation
post/get/graphQL called reject() on non-ok or non-JSON responses but didn't return, so the chain kept running and tried to parse the body anyway, potentially resolving after already rejecting.
Replace manual new Promise(...) wrapping and .then/.catch chains with async/await now that the underlying calls (fetch, throttle, resource methods) are already promise-based. Behavior is unchanged: rejections become thrown errors, still caught identically by callers' .catch().
Replace all _.each/_.map/_.filter/_.uniq/_.difference/_.intersection/ _.reduce/_.mapObject/_.object/_.extend/_.isObject/_.isArray/_.max/ _.sortBy calls with native Array/Object equivalents, and remove three dead `import _ from 'underscore'` statements that had no usages.
shelljs was pulled in for a single call, shell.mkdir('-p', dir), used
three times to create the global/cache/frame directories. Node's
fs.mkdirSync(dir, { recursive: true }) does the same without a shell
subprocess or the extra dependency weight.
The only remaining prompt usage was a single yes/no confirmation (overwrite existing report file). @inquirer/confirm matches the @inquirer/core generation already used by checkbox/select, so no duplicate @inquirer/core install, and drops prompt plus its 14 transitive dependencies.
Replaces the promise-chain waterfall of new-Promise wrappers with a flat async action. Fixes error-handling defects in the old chain: - .catch(error => done(error)).then(() => done()) called the parallel worker callback twice on failure (merge requests, timelogs) - report.getIssues() had no rejection handler, hanging the run on error - .catch(e => reject(e)).then(...) resolved after rejecting and reset the project config even on failure Behavior is unchanged: same stage order, same messages, Cli.x/Cli.error still terminate the process on fatal errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The private token was appended to every GET query string and injected into every POST body in addition to the PRIVATE-TOKEN header. Tokens in URLs leak into proxy/server logs and shell history; the header alone is sufficient for both REST endpoints (GraphQL already uses a Bearer header). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the first tests for the report aggregation (per-user/per-project totals, estimate/spent sums, free and half-price label accounting, sorting, per-day consolidation, column preparation) and for Frame (new-resource detection, start/stop file round-trip, duration, date validation). These protect the areas the ongoing refactoring touches most. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
String throws lose stack traces and break instanceof checks. GitlabClient errors now include the HTTP method, path, status and (truncated) response body, so failed syncs report the actual GitLab error instead of a bare 'response not OK'. Cli.error accepts Error instances since commands pass thrown values straight through. Also guards against responses without a content-type header. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
frame.write() used unlinkSync + appendFileSync; a crash between the two lost the frame, and a crash mid-append left a truncated JSON file. Write to <file>.tmp and rename over the target instead — rename is atomic on the same filesystem, and FrameCollection only picks up *.json, so a leftover .tmp can never be parsed as a frame. Also removes the unused assertFile() helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Moves calculate() into src/reporting/stats.js as a standalone calculateStats(config, report) function; the Output constructor assigns its result, so subclasses keep reading this.times/this.stats/etc. unchanged. Billing logic (free/half-price labels, now via Array.some/includes instead of bitwise |= loops) no longer lives in the presentation layer, and the specs test the function directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Report.getProject() and Owner.getGroup() now take the project/group path as a parameter instead of reading it from config, so the report command no longer rewrites config 'project' on every loop iteration — that was only safe because the loop ran with a single runner. Also drops the unused EventEmitter inheritance from the config class. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
parallel() now takes an async (task) => void worker (plain sync functions also work); FrameCollection/ReportCollection.forEach pass the same contract through. Removes the try/done boilerplate in every caller and fixes the remaining double-callback in Report.process(), where getStats() failures invoked done twice via .catch(done).then(done). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Class declarations now follow the PascalCase convention already used by GitlabClient/Output/Timekeeper (default exports, so no import changes; output classes are named TableOutput/CsvOutput/etc. to avoid clashing with their Table/Csv imports). ReportCollection's module-level projlist becomes an instance property, so the de-duplication no longer leaks across collection instances. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All date handling goes through src/core/dayjs.js, which configures the
utc, timezone, relativeTime (fromNow) and advancedFormat ('Do' token)
plugins once. moment is in maintenance mode and was the heaviest
dependency in the pkg bundle; dayjs is API-compatible for everything we
use (format/startOf/endOf/subtract/add/diff/isValid/isSame/isBefore/
tz/clone/fromNow, ISO JSON serialization, Sunday week start). The
GraphQL date format 'Y-M-D' becomes 'YYYY-M-D' since single 'Y' is not
a dayjs token; moment-only suppressDeprecationWarnings knobs are gone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Only eachLimit was used; parallel() now runs N runner loops pulling from a shared queue, stops starting new tasks after the first failure and rejects with that error — same contract as before, one dependency less. Covered by a new spec (all tasks processed, concurrency bound, fail-fast, sync workers, empty input). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
colors is unmaintained (and had the 2022 sabotage release); it also worked by patching String.prototype, which let several files use .red/ .magenta without importing anything. picocolors is explicit function calls (pc.red(x)), so every consumer now imports it directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
read-yaml was a decade-old wrapper; FileConfig now uses js-yaml's load with fs.readFileSync directly (same semantics, including undefined for empty files). node-spinner 0.0.4 is replaced by a four-frame local spinner in cli.js — the only thing it was used for. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
No description provided.