Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork97
Transform attribute names to proper casing#199
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
@gpoitch I think it's definitely something we should address. Regarding the closing: Did you ran into problems with this PR? I'm mainly curious to see if there is a way without changing the casing in the JSX renderer, but lowercase it in the default mode. |
@marvinhagemeister accidental auto-close referencing this PR in a private repo. Not familiar with default vs jsx mode - I'll have to read the source code more and circle back. |
@marvinhagemeister I updated it so the jsx renderer is unaffected by applying the normalization after the attribute hook intercepts the standard rendering. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/index.js Outdated
| // <textarea value="a&b"> --> <textarea>a&b</textarea> | ||
| propChildren=v; | ||
| }elseif((v||v===0||v==='')&&typeofv!=='function'){ | ||
| name=getAttributeNameInHtmlCase(name); |
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.
Likely faster if this is done on L310:
-s += ` ${name}="${encodeEntities(v)}"`;+s += ` ${getAttributeNameInHtmlCase(name)}="${encodeEntities(v)}"`;
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.
Need the transformed name for theif (v === true || v === '') block
changeset-botbot commentedNov 12, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
🦋 Changeset detectedLatest commit:61cd8c2 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 |
src/index.js Outdated
| constDASHED_ATTRS=/^(acceptC|httpE|(clip|color|fill|font|glyph|marker|stop|stroke|text|vert)[A-Z])/; | ||
| constCAMEL_ATTRS=/^(isP|viewB)/; | ||
| constCOLON_ATTRS=/^(xlink|xml|xmlns)([A-Z])/; | ||
| constCAPITAL_REGEXP=/([A-Z])/g; | ||
| constUNSAFE_NAME=/[\s\n\\/='"\0<>]/; |
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 they cover all the attributes?react-dom has an entire file forpossibleStandardNames:
If they can, at least add a unit test case to cover them all. cc@gpoitch
Uh oh!
There was an error while loading.Please reload this page.
Makes sure attributes are converted to their proper casing when rendering to string. There was a similar attempt with#55 but it was never completed. Reviving it particularly because Safari is excessively downloading an img's src upon hydration due to
srcSetcasing mismatch between preact/preact-render-to-string. This also adds better support for attributes specific to SVG/RSS/XML.Incorporated the concerns from the comments in the other attempts/issues.
Attribute list reference:https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes
React reference:https://github.com/facebook/react/blob/main/packages/react-dom/src/shared/possibleStandardNames.js
Closes#55
Closes#18