Repository navigation
feat(lint): flag a set that fires inside a running tween on the same property - #7059
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Approving at ea210de5. I found no blocker.
Checked:
- Fails without the fix: with the base
gsap.ts(d03bdeae9), the named test fails (1 failed / 235 passed). At head,gsap.test.tspasses 236/236. - False positives: I linted all 756 HTML files under
registry/,skills/andexamples/before and after the change. Both runs give 35overlapping_gsap_tweensfindings, and none of them is a new set-inside-tween finding, which matches the PR body. windowOverlap: two tweens that both have a duration are compared the same way as before. A set is reported only when it lands strictly inside the other tween's window, so a set at the tween's first frame or at its end isn't reported. Two sets are never compared. Theoverwrite: "auto"and unresolved-target skips still run first. No other rule reports the same case, so nothing is flagged twice.
Nit (not blocking): a set inside a repeat: -1 tween on the same property has running.end === Infinity, so the message reads …to Infinitys and the hint says move the set to Infinitys or later. That case is a real conflict, so the warning itself is correct. You could word the infinite case differently in a follow-up.
Contributor
Merged
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
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.
What
overlapping_gsap_tweensnow also flags atl.set()that fires while a tween on the same element and property is still running. The message names the element, the property, the set's time and the tween's window; the fix hint says to let the tween finish before the set fires.Why
The rule skipped any zero-length window, so a
set(zero duration) was never compared with a running tween. That is the shape of a one-word lyric's pop-in (fromToopacity 0 to 1 over 0.16 s) followed by a hide (setopacity 0) one frame before it ends. Played in order the set wins; seeked, as scrub and export do frame by frame, the tween can win, so the old word stays on screen.Two tweens with a duration are compared exactly as before. A set at or after the tween's end, at its first frame, on another property, or on another element is not reported.
Testing
fails without the fix: packages/lint/src/rules/gsap.test.ts > "a set that fires inside a running tween on the same property" > "reports a hide that lands in a pop-in's last frames, naming the element and both times"
On the devbox: the lint package's tests pass with the change; with the old
gsap.tsthe new test fails (1 failed, 235 passed). Linting all 756 HTML files underregistry/,skills/andexamples/gives the same 35overlapping_gsap_tweensfindings before and after, so no new false positive.Size
A small helper and a rewritten loop in one rule, with its tests; it has no other carrier.