types: Remove 'AllHTMLAttributes' from core #4715
Closed
Conversation
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
* Revert assign and avoid repeating indexed access * Check parentNode instead * Use flag * Remove type * Remove todo file
* Forward ref by default * Optimizations
* Move `defaultProps` into `preact/compat` This will be handled in `options.vnode` for function/class components. This hook gets called for every invocation of `jsx`/`createElement` and `cloneElement`. * Try it * refactor: This is horrific but seems to work? (#4662) --------- Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
* Remove unused imports * Comment denoted hydration * Make it work * Golfies
* Remove unused imports * refactor: Switch to Object.is for hook args * refactor: Copy to `useReducer` & store `Object` accessor * test: Add tests for `useEffect` & `useState` w/ `NaN` Co-authored-by: jayrobin <james.michael.robinson@googlemail.com> --------- Co-authored-by: jdecroock <decroockjovi@gmail.com> Co-authored-by: jayrobin <james.michael.robinson@googlemail.com>
This reverts commit 6b8bfa2.
* refactor: Switch to `package.json#exports.module`, drop `.min` builds, & use `.mjs` exclusively * chore: Remove leftover CJS shell * test: Fix export for karma * fix: coverage not generated in minify tests --------- Co-authored-by: Marvin Hagemeister <hello@marvinh.dev>
* Remove automatic px suffix * Remove from jsx-runtime
Member
JoviDeCroock
left a comment
JoviDeCroock
left a comment
There was a problem hiding this comment.
I'm atm very hesitant on this one due to people that might be relying on this
Member
Author
|
They shouldn't be is the point I was trying to make, it doesn't reasonably constrain the types. There's no situation that I can see in which every single known attribute/property is valid -- it's always a subset. Happy to let this sit but this (or reverting #4706, which I didn't foresee this issue with it) blocks #4607. |
JoviDeCroock
approved these changes
Mar 10, 2025
Member
Author
|
Upon more thought, let's not. The value is unclear & it's a pain to maintain correctly, but letting it sit isn't much of an issue. I'll revert part of #4706 instead. |
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.
Targetting v11 as it's breaking.
I'd like to kill off this interface from core, only offering it as
anythrough compat going forward. It's fundamentally a bad interface and no one should be using it:Using it allows for nonsensical props to be set. This is most often utilized by component libs that do something like
props: AllHTMLAttributes<HTMLDivElement>, I'm guessing expecting theHTMLDivElementgeneric to somehow narrow the type but it doesn't work like that. This allows the user to set (say)referrerPolicydespite that doing nothing on the target<div>element.There's obvious & unavoidable conflicts in the interface members, such as
InputHTMLAttributes.typevsAnchorHTMLAttributes.type-- whichtypedo we expose here? Common denominatorstring?any?It's a pain to maintain. Theoretically every single prop/attr needs to be added to that interface and we can't simply extend our per-element interfaces as they're not static (or won't be w/ refactor: Restrict aria roles by element type #4607). Every type alteration would need to be added in two separate places, else, the types start getting a bit flakey for users.
We have per-element types now, and while they're probably not perfect, users should be reaching for them instead as they're much, much more useful.