feat(nanoviews): let an effect attribute name the element it sits on - #206
Merged
Merged
Conversation
`EffectAttributeValues` takes the target element, so a value type can be about the element rather than fixed once for all of them. Attributes that do not care never mention it. `ref$` is the one that does. Its signal used to be typed `WritableSignal<Element | null>`, which did not merely fail to infer the element - it forbade naming it: a signal is invariant, so `signal<HTMLButtonElement | null>(null)` on a button was rejected outright, and the only legal spelling was `Element | null` with a cast at every use. The accepted types are now named instead: the element itself, `HTMLElement`, `SVGElement` and `Element`, so both the precise signal and the loose one fit, and a signal of some other element does not. Types only - the runtime is untouched and no bundle moves.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #206 +/- ##
=======================================
Coverage 85.32% 85.32%
=======================================
Files 139 139
Lines 3142 3142
Branches 591 591
=======================================
Hits 2681 2681
Misses 332 332
Partials 129 129 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
An effect attribute declares the type of the value it takes:
Written once, for every element. For
ref$that is not merely imprecise — it is wrong in a way that costs the consumer something. A signal is invariant, soWritableSignal<HTMLButtonElement | null>is not aWritableSignal<Element | null>, and naming the element you are referencing was rejected:So the only legal spelling was
signal<Element | null>(null), and every read of it needed a cast to reach anything the element actually has.The map takes the target
PickEffectAttributesByTargetalready knew the element — it uses it to decide whether an attribute applies. Now it passes it on to decide what the attribute takes. Attributes stay in one map and keep declaring themselves from their own file; the four that do not care about the element (classList$,style$,autoFocus$, the controls) only repeat the parameter list that augmenting a generic interface requires.What
ref$says nowNaming the arms is not laziness — TypeScript has no way to say "any supertype of
Target". Two shapes that would avoid the list were tried and neither works. A type-level lambda (<T extends Element>(target: T) => WritableSignal<T | null>applied throughinfer) does not substitute the argument: TypeScript erases the parameter to its constraint, and<T>(t: T) => T[]applied tonumbercomes backunknown[]. A structural sink ((value: Target | null) => void, "anything the element can be written into") accepts everything, including a signal of a different element — a signal's getter overload takes no arguments, and a zero-argument function satisfies any one-argument target.What this buys
signal<HTMLElement | null>andsignal<Element | null>still fit, so nothing that compiled before stops compiling.signal<HTMLInputElement | null>on a button does not, and the test asserting that is a real gate: it uses@ts-expect-error, so if the check ever stops working the suppression becomes unused andtscfails.Types only. The runtime is untouched and every bundle is byte-identical.