BuilderIO / mitosis

Write components once, run everywhere. Compiles to React, Vue, Qwik, Solid, Angular, Svelte, and more.
https://mitosis.builder.io
MIT License
12.55k stars 558 forks source link

`htmlFor` does not transpile properly in none-JSX frameworks #916

Closed rootEnginear closed 1 year ago

rootEnginear commented 2 years ago

I am interested in helping provide a fix!

Yes

Which generators are impacted?

Reproduction case

https://mitosis.builder.io/?outputTab=G4VwpkA%3D&code=KYDwDg9gTgLgBAE2AMwIYFcA29noHYDGMAlhHnALICeAwhALaR7B4wAUYUEYAzgJRwA3gCg4cKMBjoo5NqLFwAPAmIA3AHzyFS4njDp4xBAF4ARKlNwA9Ju1jFmVACNgmOAAsY9TADFoZi3UAQUUrRxdMWwVQlQ15PgBuYQBfIA%3D

Expected Behaviour

<template>
  <div>
    <input id="a" />
    <label for="a">A</label>
  </div>
</template>

Actual Behaviour

<template>
  <div>
    <input id="a" />
    <label htmlFor="a">A</label>
  </div>
</template>

Additional Information

(might be related to #526) In none-JSX frameworks (please tell me if I'm incorrect), htmlFor attribute should be transpiled as for.

samijaber commented 1 year ago

Thanks for filing this bug, and being interested in contributing a fix! This one is a good first issue :)

Easiest way to fix this for all generators is to:

Add logic in the JSX parser to map the htmlFor key to for

Now, the Mitosis JSON will store it as for everywhere. This means it works for all non-JSX frameworks. We will need to update the JSX generators (SolidJS, React, Qwik) to map for back to htmlFor for it to work correctly there.

React

React has a binding mapper object to do this:

https://github.com/BuilderIO/mitosis/blob/24688615f284a58c256211147cdf85591be182cf/packages/core/src/generators/react/generator.ts#L148-L175

You can add a one-liner to ATTRIBUTE_MAPPER i.e. 'for': 'htmlFor'

SolidJS

SolidJS doesn't have one. The logic for both properties and bindings is here:

https://github.com/BuilderIO/mitosis/blob/24688615f284a58c256211147cdf85591be182cf/packages/core/src/generators/solid/index.ts#L192-L197

Will likely be best to add a BINDING_MAPPER dictionary that is checked in both loops (like the React generator), and contains a mapping from for to htmlFor.

Qwik

Qwik has this logic that runs for both bindings/properties: https://github.com/BuilderIO/mitosis/blob/24688615f284a58c256211147cdf85591be182cf/packages/core/src/generators/qwik/src-generator.ts#L350-L352

Need to add a similar line to the if (key === 'innerHTML') key = 'dangerouslySetInnerHTML'; one, but for htmlFor/for.

rootEnginear commented 1 year ago

Closed with #943