Skip to content

feat: add CronIterator — ECMAScript-native stateful iterator for Cron schedules#365

Open
Copilot wants to merge 11 commits intodevfrom
copilot/add-ecmascript-iterator-to-cron
Open

feat: add CronIterator — ECMAScript-native stateful iterator for Cron schedules#365
Copilot wants to merge 11 commits intodevfrom
copilot/add-ecmascript-iterator-to-cron

Conversation

Copy link
Contributor

Copilot AI commented Mar 5, 2026

  • Implement CronIterator<T> class in src/croner.ts with Iterator + Iterable protocols, .next(), .peek(), .reset(), and private _normalizeCursor() helper
  • Add Cron.prototype.enumerate(startAt?) factory method — placed alongside nextRun() / nextRuns() / previousRuns() as a query method
  • Export CronIterator from src/croner.ts
  • Fix CronIterator.next() cursor drift when dayOffset is set — cursor is stored as the un-offset schedule time; yielded Date is always a fresh clone
  • Fix CronIterator string startAt timezone parsing — strings are now preserved as-is so nextRun() can parse them via CronDate/fromTZISO using the Cron instance's configured timezone
  • Add 20 tests in test/iterator.test.ts including dayOffset and timezone string-startAt consistency tests against nextRuns()
  • Update docs/src/usage/basics.md with enumerate API
  • Update docs/src/usage/examples.md with iterator examples
  • Update README.md with enumerate entry in the status section
  • Run pre-commit checks (fmt, lint, type-check) — all pass
  • Run full test suite — 558 passed, 0 failed

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@Hexagon Hexagon added this to the 11.0 milestone Mar 5, 2026
…native iteration

Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Copilot AI changed the title [WIP] Add ECMAScript-compliant stateful iterator to Cron class feat: add CronIterator — ECMAScript-native stateful iterator for Cron schedules Mar 5, 2026
@Hexagon
Copy link
Owner

Hexagon commented Mar 5, 2026

@copilot Just to check. Does this follow common practice? Does this fallow croner practice? Is this documented the same way as existing features? Are the changes minimal? Is it documented the same way as existing features? Is it tested properly and efficient?

Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 5, 2026

@copilot Just to check. Does this follow common practice? Does this fallow croner practice? Is this documented the same way as existing features? Are the changes minimal? Is it docum...

Reviewed everything against the existing codebase — here's what was corrected in commit 010773e:

Common practice: CronIterator correctly implements Iterator<Date, undefined> + Iterable<Date>, with [Symbol.iterator]() { return this; } — the standard pattern for a combined iterator/iterable in ECMAScript.

Croner practice:

  • enumerate() was misplaced near the end of the Cron class; it's now grouped directly with nextRun(), nextRuns(), and previousRuns() where it belongs as a query method.
  • Test file had // --- section dividers not found anywhere in croner's test suite — removed. Tests now follow the same flat, no-divider style as previous-runs.test.ts and options.test.ts.

Documentation: Consistent with existing features — single-line entry in the Status section of both README.md and basics.md, plus a standalone example section in examples.md (same pattern used for previousRuns).

Minimal changes: CronIterator adds ~70 lines (class + jsdoc), enumerate() is a 3-line factory, and the test file is 18 focused tests. No changes to existing public API.

Testing: All tests now use exact .toISOString() assertions (matching previous-runs.test.ts style), no unused variables, no weak >= 1 comparisons. 556 total tests pass, 0 failed.

@Hexagon Hexagon marked this pull request as ready for review March 5, 2026 23:30
@Hexagon Hexagon requested a review from Copilot March 5, 2026 23:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an enumerate() API to produce a stateful, ECMAScript-native iterator over upcoming cron occurrences, along with tests and documentation updates to demonstrate usage across the library.

Changes:

  • Added CronIterator<T> (Iterator + Iterable) with next(), peek(), and reset() in src/croner.ts, plus Cron.prototype.enumerate(startAt?).
  • Added a new iterator-focused test suite covering iterator protocol behavior and parity with nextRuns().
  • Updated docs/README to document the new enumerate() API and provide examples.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/croner.ts Introduces CronIterator and Cron.enumerate() factory, and exports the iterator.
test/iterator.test.ts Adds coverage for iterator semantics (next/peek/reset, iterable behavior, stopAt exhaustion).
docs/src/usage/basics.md Documents enumerate() in the “Status” API list.
docs/src/usage/examples.md Adds example usage for enumerate() with for...of, destructuring, peek, reset, and stopAt.
README.md Adds enumerate() to the README “Status” section API list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Hexagon
Copy link
Owner

Hexagon commented Mar 5, 2026

@copilot apply changes based on the comments in this thread

Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 5, 2026

@copilot apply changes based on the comments in this thread

Both issues from the review thread are addressed in commit 9812f0a:

Cursor drift with dayOffset (src/croner.ts:790-796): next() now reverses the dayOffset before storing the cursor, matching exactly what _enumerateRuns does (it keeps currentCron = nextCron — the raw un-offset CronDate — and only applies offset on output). The fix:

const dayOffset = this.cron.options.dayOffset;
this.cursor = (dayOffset !== undefined && dayOffset !== 0)
  ? new Date(nextDate.getTime() - dayOffset * 24 * 60 * 60 * 1000)
  : nextDate;

Missing dayOffset test (src/croner.ts:786-797): Added "enumerate() with dayOffset produces the same dates as nextRuns()" which uses dayOffset: 1 and asserts all 5 iterated dates are byte-for-byte identical to nextRuns(5). 557 tests pass.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/croner.ts Outdated
Comment on lines +774 to +777
private static _normalizeDate(d?: Date | string | null): Date | undefined {
if (d === undefined || d === null) return undefined;
return d instanceof Date ? new Date(d.getTime()) : new Date(d);
}
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CronIterator._normalizeDate() converts string inputs to new Date(d), which changes semantics when the Cron instance is configured with timezone (CronDate.fromString uses fromTZISO(str, tz) to interpret the string in that timezone, while new Date(str) interprets it in the host environment timezone). This makes enumerate("...") potentially diverge from nextRun("...") / nextRuns(..., "...") for timezone-aware schedules. Consider preserving string inputs (store cursor as Date | string | undefined), or normalizing via CronDate with cron's tz so string parsing matches the rest of the API; adding a regression test with { timezone: "Europe/Stockholm" } would help.

Copilot uses AI. Check for mistakes.
@Hexagon
Copy link
Owner

Hexagon commented Mar 6, 2026

@copilot apply changes based on the comments in this thread

Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 6, 2026

@copilot apply changes based on the comments in this thread

Fixed in commit 831a466.

_normalizeDate() is renamed to _normalizeCursor() with return type Date | string | undefined. Date objects are still cloned to prevent external mutation, but strings are now preserved as-is so that nextRun() can parse them through CronDate(str, tz)fromTZISO(str, tz) — the same timezone-aware path used by nextRun() and nextRuns().

Added "enumerate() with timezone: string startAt is parsed in the Cron timezone" — uses "2024-01-01T12:00:00" (no Z/offset) with { timezone: "Europe/Stockholm" }, asserts all 5 dates match nextRuns(5) exactly, and verifies the first occurrence is "2024-01-01T12:00:00.000Z" (13:00 Stockholm = 12:00 UTC, confirming the string was interpreted in the Stockholm timezone). 558 tests pass.

@Hexagon Hexagon requested a review from Copilot March 6, 2026 20:36
@Hexagon Hexagon changed the base branch from master to dev March 6, 2026 20:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Hexagon and others added 5 commits March 6, 2026 23:06
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Hexagon
Copy link
Owner

Hexagon commented Mar 6, 2026

@copilot verify that formatting and tests are ok

Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 6, 2026

@copilot verify that formatting and tests are ok

Fixed in commit 52a6d93. There was one formatting issue — extra parentheses in (dayOffset ?? 0) flagged by deno fmt. Removed them; pre-commit (fmt + lint + type-check) and all 558 tests now pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants