Transform attribute names to proper casing #199
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. |
gpoitch
changed the title
gpoitch
changed the title
src/index.js
Outdated
| @@ -281,6 +287,8 @@ function _renderToString(vnode, context, opts, inner, isSvgMode, selectValue) { | |||
| // <textarea value="a&b"> --> <textarea>a&b</textarea> | |||
| propChildren = v; | |||
| } else if ((v || v === 0 || v === '') && typeof v !== 'function') { | |||
| name = getAttributeNameInHtmlCase(name); | |||
There was a problem hiding this comment.
Likely faster if this is done on L310:
- s += ` ${name}="${encodeEntities(v)}"`;
+ s += ` ${getAttributeNameInHtmlCase(name)}="${encodeEntities(v)}"`;There was a problem hiding this comment.
Need the transformed name for the if (v === true || v === '') block
🦋 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
| const DASHED_ATTRS = /^(acceptC|httpE|(clip|color|fill|font|glyph|marker|stop|stroke|text|vert)[A-Z])/; | ||
| const CAMEL_ATTRS = /^(isP|viewB)/; | ||
| const COLON_ATTRS = /^(xlink|xml|xmlns)([A-Z])/; | ||
|
|
||
| const CAPITAL_REGEXP = /([A-Z])/g; | ||
|
|
||
| const UNSAFE_NAME = /[\s\n\\/='"\0<>]/; |
There was a problem hiding this comment.
Can they cover all the attributes? react-dom has an entire file for possibleStandardNames:
If they can, at least add a unit test case to cover them all. cc @gpoitch
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