Guard the clock seam, and route the last direct reads through it - #6022
Draft
backspace wants to merge 3 commits into
Draft
Guard the clock seam, and route the last direct reads through it#6022backspace wants to merge 3 commits into
backspace wants to merge 3 commits into
Conversation
Ten remaining reads in this package went straight to the wall clock: a workspace's elapsed-time arithmetic, a quarter field's current-year default, time-slot generation, a date parser's reference date, a date range's today. They now go through `helpers/clock` like the renderers already did, so a test that pins the instant pins all of it, and the values stay comparable instead of having to be hidden. Routing the defaults and the parse reference is not the same argument as routing the renderers — nothing renders them as an age — but a pinned clock makes them deterministic under test for free, and leaving two of the eleven outside the seam would mean the guard below could not be a flat rule. That guard is a script rather than a lint rule because ESLint does not run over this package at all: `lint` here is ember-template-lint plus custom checks. It follows check-no-isused-option.mjs, which is the same shape of problem solved the same way. Comments are blanked before matching, since a comment describing where a timestamp came from is not a violation, and blanking rather than dropping keeps the reported line numbers true. `new Date(value)` is left alone; only the argumentless form reads the clock. Verified against each form it has to separate: both call shapes are caught, an argument-taking `new Date` is not, `performance.now()` and a `.Date.now()` property access are not, and a mention inside a line, block or Glimmer comment is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 2h 6m 38s ⏱️ + 13m 43s Results for commit 1776ce9. ± Comparison against earlier commit 4f16ed0. Realm Server Test Results 1 files ±0 190 suites ±0 1h 6m 10s ⏱️ - 2m 37s Results for commit 1776ce9. ± Comparison against earlier commit 4f16ed0. |
Two indexing tests assert the exact set of modules a card instance pulls in, and routing card code through `helpers/clock` puts one more module in that graph. The lists now carry it. The assertion is doing its job here: a module appearing in the dependency closure of every card that touches a date is a real consequence of the seam, and worth having to acknowledge rather than absorb silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`no-restricted-syntax` expresses this rule directly, so the guard is two AST selectors instead of a file walker with a hand-rolled comment stripper — and it is accurate where the regex could only approximate. The script matched line text, so it had to blank comments first to avoid flagging prose that discusses `Date.now()`, and it still flagged any string containing the call. A file whose only mention of the clock is `'call Date.now() to get the time'` reports two violations under the script and none under ESLint. Selectors also read the shape rather than the spelling: `new Date(value)` is untouched because parsing a known instant is not a clock read, and the zero-argument form is matched by `arguments.length=0` rather than by a `\(\s*\)` pattern that a line break would defeat. The reason the script existed — that ESLint does not run over this package — was true but not a constraint. `packages/catalog` is card source of the same kind and is fully linted, `ember-eslint-parser` and all, so the gap here was an oversight rather than a limitation. Three `eslint-disable` comments already in this package name rules that were never loaded, including one explaining why a WebGL paint loop may own its own frame callback: they have been inert. So this config is narrow on purpose. `root: true` with no `extends` means no rule is on beyond the clock, which keeps the guard from waiting behind a backlog that has nothing to do with it. The plugins those existing directives name are registered without their rules enabled, so the comments resolve and keep their meaning for whenever the rules are switched on. `--report-unused-disable-directives` is left off for the same reason: with almost nothing enabled, every one of them reads as unused. The selectors live in `eslint/` beside the erasable-syntax and data-test sets, which packages with their own `root: true` config already share.
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.
This is meant to discourage use of
new Date()in the base realm, using thenow()helper from #6012. This should prevent spurious Percy diffs like the one in #5986.Claude: Ten remaining reads in this package went straight to the wall clock: a workspace's elapsed-time arithmetic, a quarter field's current-year default, time-slot generation, a date parser's reference date, a date range's today. They now go through
helpers/clocklike the renderers already did, so a test that pins the instant pins all of it, and the values stay comparable instead of having to be hidden.Routing the defaults and the parse reference is not the same argument as routing the renderers — nothing renders them as an age — but a pinned clock makes them deterministic under test for free, and leaving two of the eleven outside the seam would mean the guard below could not be a flat rule.
That guard is a script rather than a lint rule because ESLint does not run over this package at all:
linthere is ember-template-lint plus custom checks. It follows check-no-isused-option.mjs, which is the same shape of problem solved the same way. Comments are blanked before matching, since a comment describing where a timestamp came from is not a violation, and blanking rather than dropping keeps the reported line numbers true.new Date(value)is left alone; only the argumentless form reads the clock.Verified against each form it has to separate: both call shapes are caught, an argument-taking
new Dateis not,performance.now()and a.Date.now()property access are not, and a mention inside a line, block or Glimmer comment is not.