Skip to content

Polyfill: Replace unreachable retry with assertion in NudgeToCalendarUnit#3291

Open
jessealama wants to merge 1 commit intotc39:mainfrom
jessealama:nudge-sign-negative-dead-code
Open

Polyfill: Replace unreachable retry with assertion in NudgeToCalendarUnit#3291
jessealama wants to merge 1 commit intotc39:mainfrom
jessealama:nudge-sign-negative-dead-code

Conversation

@jessealama
Copy link
Collaborator

The retry path for negative durations appears unreachable: end-of-month constraining reduces the day, which for negative durations pushes the endpoint further from the origin, so the nudge window always contains the destination.

This isn't a rigorous proof of unreachability, but we haven't been able to construct a test case that exercises this path. Replace the retry block with an assertion guarding the invariant, so that if we're wrong, we'll get a clear error rather than silent misbehavior.

Closes #3235

…Unit

The retry path for negative durations appears unreachable: end-of-month
constraining reduces the day, which for negative durations pushes the
endpoint further from the origin, so the nudge window always contains
the destination.

Closes tc39#3235
@jessealama jessealama requested a review from ptomato March 5, 2026 12:58
@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.57%. Comparing base (be4e0ef) to head (ff81f17).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3291      +/-   ##
==========================================
+ Coverage   97.40%   97.57%   +0.16%     
==========================================
  Files          22       22              
  Lines       10771    10757      -14     
  Branches     1866     1865       -1     
==========================================
+ Hits        10492    10496       +4     
+ Misses        258      240      -18     
  Partials       21       21              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jessealama
Copy link
Collaborator Author

This change was originally in #3289 (where it was simply deleted, without even an assertion in place).

@ptomato
Copy link
Collaborator

ptomato commented Mar 5, 2026

I think if we assume this code can't be reached by any JS code, we should change the spec text accordingly to have an assertion there as well, and submit it as an editorial change. (Could possibly also be done after stage 4.)

I have a hunch that the reason the nudge window only needs to be retried when moving forwards, is because the reason for the retry is when the edge of the window lands in a DST gap. The disambiguation: "compatible" behaviour always picks the later time for DST gaps, so that might be why. It's just a hunch, but maybe that helps ruling out why this doesn't happen?

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.

Test coverage gap in NudgeToCalendarUnit

2 participants