-
-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: spellcheck
prop
#393
fix: spellcheck
prop
#393
Conversation
🦋 Changeset detectedLatest commit: 6255591 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Oh what the heck happened to the lockfile, will fix that later. Gotta get some food |
src/lib/util.js
Outdated
export const HTML_LOWER_CASE = /^accessK|^auto[A-Z]|^cell|^ch|^col|cont|cross|dateT|encT|form[A-Z]|frame|hrefL|inputM|maxL|minL|noV|playsI|popoverT|readO|rowS|spellC|src[A-Z]|tabI|useM|item[A-Z]/; | ||
export const SVG_CAMEL_CASE = /^ac|^ali|arabic|basel|cap|clipPath$|clipRule$|color|dominant|enable|fill|flood|font|glyph[^R]|horiz|image|letter|lighting|marker[^WUH]|overline|panose|pointe|paint|rendering|shape|stop|strikethrough|stroke|spel|text[^L]|transform|underline|unicode|units|^v[^i]|^w|^xH/; | ||
export const HTML_LOWER_CASE = /^accessK|^auto[A-Z]|^cell|^ch|^col|cont|cross|dateT|encT|form[A-Z]|frame|hrefL|inputM|maxL|minL|noV|playsI|popoverT|readO|rowS|src[A-Z]|tabI|useM|item[A-Z]/; | ||
export const HTML_ENUMERATED = /^dra|spel/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a set instead, performance wise this could help a lot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, can do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, what makes a set preferable here and not for the others? We certainly do make use of the patterns in a couple places, but that should just be byte saving, no?
There are certainly are a few more boolean props -> enumerated attrs, if it's the size of 2 that is what immediately concerns you, though I don't know quite how many there are. Will have to go hunting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have undergone the efforts to port self-closing to a set but for the others I just don't know the amount of cases 😅
Also for style it's mainly keeping inline with Preact core because this regex is already lacking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, thanks!
eb7cc67
to
62ade27
Compare
Supports
spellcheck={false}
& removes support forspellCheck
as we don't support it in Preact (see: preactjs/preact#3399)Related: #388, denoland/fresh#2649, preactjs/preact#4497
This adds a
HTML_ENUMERATED
rgx to stuff enumerated true/false attributes into in the future, as I brought up in #388.