BUG: Mitosis does not camelCase HTML attributes in React

[samijaber]

Scope

• [ ] This impacts ALL Mitosis outputs
• [x] This only impacts specific generators/outputs (please list them here): React

Describe the bug Attributes like srcset are camelCased in React (possibly other frameworks? Not sure). However, Mitosis does not account for that

To Reproduce

A link to a https://mitosis.builder.io/ fiddle containing the bug: link

Expected behavior scrset should become srcSet in the React output. Same goes for all HTML attributes that are multi-word: https://reactjs.org/docs/dom-elements.html

Additional context

• I am not sure what the comprehensive list of camelCased DOM elements is, but we can probably look at React's JSX types to determine what to do
• I am also not sure if other frameworks do this camelCasing. We might need to do this elsewhere too

[samijaber]

Technical information:

We already have a list of all HTML elements: https://github.com/BuilderIO/mitosis/blob/b8d083ca78bdaec332745d2a0f8c0890335072c4/packages/core/src/constants/html_tags.ts#L1

We want to go to the blockToReact function, and identify blocks that are native HTML elements, and rename any json.bindings or json.properties keys that should be camelCased.

https://github.com/BuilderIO/mitosis/blob/b8d083ca78bdaec332745d2a0f8c0890335072c4/packages/core/src/generators/react.ts#L164-L167

https://github.com/BuilderIO/mitosis/blob/b8d083ca78bdaec332745d2a0f8c0890335072c4/packages/core/src/generators/react.ts#L184-L186

In both of these for-loops, we want to add a check that looks like this:

const isHtmlElement = VALID_HTML_TAGS.includes(json.name);

const checkKeyShouldBeCamelCased = (key: string) => {
  // ... TO-DO:
}

const camelCaseNativeHTMLAttribute = (attrName: string) => {
  // ... TO-DO:
}

if (isHtmlElement && checkKeyShouldBeCamelCased(key)) {
  str += ` ${camelCaseNativeHTMLAttribute(key)=${value}`
} else if // the rest of the code

[shubham-y]

I will like to take up this issue

[steve8708]

oh I'm not sure if lists are the wistest approach here, HTML spec constantly evolves. The intent behind our JSX is that we match React JSX, so srcSet should be srcSet in Mitosis too

In the early days I tried to do transformations like this (and we explored it with Qwik for a minute), but it wasn't wise

what's the concern with just writing <picture srcSet> in mitosis? then we lowercase them

React's logic is similarly intentionally simple, there is no trying to replicate DOM tags and attrs in the library, their logic (which the intent behind Mitosis is)

const isHtmElement = isFirstLetterLowerCase(json.name)

[steve8708]

looking a little deeper, I feel a bit more reason to copy react the convention. with all of this we need to account for webcomponents too. what I've observed react's full logic is:

const isHtmlElement = isFirstLetterLowerCase(json.name)
const isBuiltInElement = !json.name.includes('-') 
const isCustomElement = json.name.includes('-')

const isBuiltInHtmlAttrName = isHtmlElement && !propertyName.include('-')
const isCustomHtmlAttrName = isHtmlElement && propertyName.includes('-')

but in general we should stick to if Mitosis uses JSX style (camelCase properties) or HTML style. for familiarity, my belief was JSX style is the right way, with one exception for class - similar to what Solid does.

[samijaber]

The intent behind our JSX is that we match React JSX, so srcSet should be srcSet in Mitosis too

I hadn't realized we wanted to match React more closely than the other ones. Thanks for the context

what's the concern with just writing <picture srcSet> in mitosis? then we lowercase them

No concern, we could do it one way or the other! My thinking was that since every non-React framework I looked at sticks with the original syntax (Solid supports both srcSet and srcset btw), it'd be good to stick with the native casing as React is the odd one out (do a transformation in 1 generator vs in all-but-1)

- oh I'm not sure if lists are the wistest approach here, HTML spec constantly evolves
- there is no trying to replicate DOM tags and attrs in the library

Worth noting that either way, we'll have to manually add every new HTML tag & attribute to our JSX type, so we are constantly maintaining a mirror of the HTML spec regardless. I'd rather stick with the list that's already hardcoded for this issue, and I can go in and replace it with a isFirstLetterLowerCase(json.name) check in a follow-up PR if we feel strongly about this.

New Approach

@shubham-y , hold off on working on this temporarily, I'll add a comment later with details about the new approach we want to take to address this.

[steve8708]

super interesting that solidjs supports both. I personally like that approach if it's not a logistical headache on the implementation side

my main suggesting is just avoiding hardcoded lists of element tags/properties however possible. all frameworks need to do that for JSX types, but I've never seen it be built into the frameworks itself and affecting logic in these types of ways. I think the main reason why is it's ok if a type is wrong or incomplete (you can override it locally or use any), but it's not ok if a framework makes a wrong assumption internally

so types can be seen as a nice to have (that you can override/augment), but framework logic you can't override and needs to support more than just what is in the HTML spec today (including custom elements, custom attributes, etc) in a predictable way

[samijaber]

@shubham-y Sorry for the delay here! Wanted to chime in with an overall idea for the new approach:

Instead of making the initial HTML casing our default (e.g. srcset), we want to go with camelCasing (e.g. srcSet). This approach will work for React by default, but we will need to:

• create a mapCamelCasedHtmlAttribute function that is reused by all code generators except the React one.
• change our JSX runtime types to remove all the regular HTML casing versions, like these (there are many more): https://github.com/BuilderIO/mitosis/blob/dae644a2b6a38e3b25936937717d520612951409/packages/core/jsx-runtime.d.ts#L454 https://github.com/BuilderIO/mitosis/blob/dae644a2b6a38e3b25936937717d520612951409/packages/core/jsx-runtime.d.ts#L448 https://github.com/BuilderIO/mitosis/blob/dae644a2b6a38e3b25936937717d520612951409/packages/core/jsx-runtime.d.ts#L256

The function I'm describing will look roughly like what we've described in this conversation:

/*
 * This function does side-effects on `json`
 */
const mapCamelCasedHtmlAttributes = (json: MitosisNode) => {
  const isBuiltInHtmlElement = isFirstLetterLowerCase(json.name) && !json.name.includes('-');

  if (!isBuiltInHtmlElement) {
    return;
  }

  for (key in json.bindings) {
    const isBuiltInHtmlAttrName = !key.include('-')
    if (isBuiltInHtmlAttrName) {
      const newKey = key.toLowerCase();
      // if json[newKey] already exists: show a warning and don't override
      json[newKey] = json[key];
      delete json[key];
    }
  }  

  // repeat the above loop for json.properties too
}

And then you'll want to add this logic to the start of every generator, like here:

https://github.com/BuilderIO/mitosis/blob/dae644a2b6a38e3b25936937717d520612951409/packages/core/src/generators/svelte.ts#L311-L334

A good place would be right after the runPostJsonPlugins call of each generator.

I think this should work: the snapshot tests will help you (you might have to tweak a snapshot test or add one that shows this transformation). Let me know if you need any additional help! 🙏🏽