Skip to content

fix execEvenlyMinDelayMs not reflecting points/duration mutation - #364

Merged
animir merged 2 commits into
animir:masterfrom
quad:fix-execevenly-min-delay-mutation
Jun 5, 2026
Merged

fix execEvenlyMinDelayMs not reflecting points/duration mutation#364
animir merged 2 commits into
animir:masterfrom
quad:fix-execevenly-min-delay-mutation

Conversation

@quad

@quad quad commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

The setter evaluated the default at first assignment and cached it, so
runtime updates to points or duration left the spacing computed from
construction-time values. The docs state that options can be changed at
runtime; this brings execEvenlyMinDelayMs in line with that contract.

Now: if execEvenlyMinDelayMs was never explicitly set, the getter
derives it from the current msDuration/points on each read. Explicit
values are still honoured.

The setter evaluated the default at first assignment and cached it, so
runtime updates to points or duration left the spacing computed from
construction-time values. The docs state that options can be changed at
runtime; this brings execEvenlyMinDelayMs in line with that contract.

Now: if execEvenlyMinDelayMs was never explicitly set, the getter
derives it from the current msDuration/points on each read. Explicit
values are still honoured.
Copilot AI review requested due to automatic review settings June 1, 2026 11:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates execEvenlyMinDelayMs to be lazily derived from current points/duration unless explicitly set, and adds tests to ensure the derived value updates when inputs are mutated.

Changes:

  • Make execEvenlyMinDelayMs getter derive from msDuration / points when no explicit value is set.
  • Simplify execEvenlyMinDelayMs setter to store the provided value as-is.
  • Add unit tests covering re-derivation behavior when points/duration are mutated.

Reviewed changes

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

File Description
test/RateLimiterAbstract.test.js Adds tests validating derived vs explicit execEvenlyMinDelayMs behavior under mutation.
lib/RateLimiterAbstract.js Changes execEvenlyMinDelayMs to be computed dynamically unless explicitly set.

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

Comment thread lib/RateLimiterAbstract.js
Comment thread lib/RateLimiterAbstract.js
Comment thread lib/RateLimiterAbstract.js
@animir

animir commented Jun 5, 2026

Copy link
Copy Markdown
Owner

@quad Hey. Looks good to me. I've added some guards for edge cases. Thanks.

@animir
animir merged commit b722d13 into animir:master Jun 5, 2026
3 checks passed
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