fix dropdown-menu commandFor casing and date-picker doc typo#90
Open
fix dropdown-menu commandFor casing and date-picker doc typo#90
Conversation
✅ Deploy Preview for kinu-sh ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Size Change: -3 B (-0.02%) Total Size: 14 kB
ℹ️ View Unchanged
|
Owner
|
I like option 2 |
- aspect-ratio, avatar: remove unnecessary BaseProps extension which leaked command/commandFor onto native elements causing null assignability errors against Preact's Signalish types - types.d.ts: augment HTMLAttributes with command/commandfor/commandFor so custom attributes are accepted on any native element - carousel: destructure command/commandFor from props before spreading to prevent BaseProps nullables from conflicting with button attributes - dropdown-menu: use lowercase commandfor on <dialog> (camelCase commandFor does not exist on DialogHTMLAttributes) - docs/components/date-picker.md: escape pipe characters in prop table value type so the markdown table renders correctly
The previous commit fixed type errors by modifying component files and the global type augmentation. Those changes were wrong in intent — they removed BaseProps from components that should have it, silently dropped props in carousel, and widened the global augmentation. The actual root cause was BaseProps allowing null on command and commandFor, which Preact's Signalish type rejects. Remove the null from both props in component-props.ts and revert all other changes.
The | null on command and commandFor was added intentionally by the author. Revert to the original.
1715c5e to
930a967
Compare
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
2 participants
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.
Two small fixes.
dropdown-menu/index.tsx—commandFor(camelCase) does not exist onDialogHTMLAttributes. Changed to lowercasecommandforto match the HTML attribute.docs/components/date-picker.md— Pipe characters in thevalueprop type were unescaped, breaking the markdown table rendering. Escaped as\|.Unresolved type errors (4)
All four share the same root cause:
BaseProps.commandandBaseProps.commandForare typed with| null, but Preact'sSignalish<string | undefined>(which wraps all HTML attribute types to support signals) doesn't acceptnull. Affectsaspect-ratio,avatar, andcarousel(×2) — all components whoseOwnPropsextendBasePropsand spread onto a native element.Options:
| nullfromBaseProps— but this was added intentionally (commit 7f14577), presumably to support null-clearing signals.preact.JSX.HTMLAttributesto includecommandandcommandFortyped asstring | null, so the global attribute types accept null. Least invasive.Signalishtype is updated to acceptnull.