BuilderIO / mitosis

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

Extra code generation in Solid loops #725

Open ryansolid opened 2 years ago

ryansolid commented 2 years ago

I am interested in helping provide a fix!

No

Which generators are impacted?

Reproduction case

https://mitosis.builder.io/?outputTab=M4ewNglgJkA%3D&code=FAUwHgDg9gTgLgAgCYgGYEMCuAbRrMB2AxnAJZQEICyAngMJQC20BIBcAFBDFBAM4BKBAG9gCBEQp9E2UtIQBeBAG1hCUkgBcCAIwAaBGTjYQ2gEQARKAkY0JAC1gg%2BZgL4HhG7QCYDRk%2BYA6jCkcCAIAFLoAG7oAMpEIRCIAGIw6IwgAO6wANZuALrAYggwIHCYMJQcJeIAPEik0QB8teIistIAdIzoEBwcoSCMQgqt7RMIDU0IuSA0Cp5hjF0ars1Lw13%2BIK51APSNLW3iAq5tB0fjCAIA3MCuQA%3D%3D

Expected Behaviour

Index is not included in the example. And key is not applied in Solid.

Actual Behaviour

Index is getting called and assigned to a local variable even though it is not used. And the key prop is being applied in Solid.

Additional Information

Solid doesn't need keys on loops, and actually de-optimizes if map is passed a function that has the index argument. Ideally I'd expect key props not to be added, and if any variables are only used in them (like an index) should be dead code eliminated.

I found this issue while looking at the output from https://github.com/BuilderIO/framework-benchmarks.

samijaber commented 2 years ago

Thanks @ryansolid! This is all helpful

ryansolid commented 2 years ago

I do recognize there are challenges here because of semantic differences. Like if someone in fact wanted to be keyed in this way then this is pretty awkward, because they'd be like I wrote the key why is this not working. And the way to properly do keyed updates in Solid when references change is quite different. For the most part people won't need the key to get keyed updates but I'm just acknowledging exception cases. But including it does nothing so probably best not to.

samijaber commented 2 years ago

Like if someone in fact wanted to be keyed in this way then this is pretty awkward, because they'd be like I wrote the key why is this not working.

But like you said, the concept of a key does not exist in Solid at all, does it? Or are there some special circumstances where Solid supports it?

Mitosis also has a highly flexible plugin +metadata system that allows folks to tweak generated code based on their needs. If people complain about key disappearing in Solid, we can easily bake in a config that the user provides, that will forcibly persist key in the Solid generator instead of ignoring it. 👍🏽