Bug: slots with data

[Marvin-Brouwer]

I need a slot in a list in which I can pass around the data of it's parent. So for example, with a component like this:

import { Slot, Layout, For } from "@builder.io/mitosis";

export default function MyComponent(props) {
  const example = [ 

    "asdf",
    "dsfdsdf"
  ]

  return (
    <div>
      
      <For each={example} >
        {
          (item) => (
            <Slot name={props.slotPage} item={item}></Slot>
          )
        }
      </For>
    </div>
  );
}

I'd like this ouput in Vue

<template>
  <div>
    <template :key="index" v-for="(item, index) in example">
      <slot name="page" :item={item}></slot>
    </template>
  </div>
</template>

<script>
import { defineComponent } from "vue";

export default defineComponent({
  name: "my-component",
});
</script>

In vue you can do: <slot name="page" :item={item}></slot>, however I can't seem to figure out how to make mitosis do this. The documentation doesn't state this as a gotcha, but it also doesn't state how to do this.

If I need to make a feature request for this, please just tell me and close the ticket.

Thanks,

---

https://mitosis.builder.io/?outputTab=G4VwpkA%3D&code=JYWwDg9gTgLgBAbzgZQDYRgGjgGQIYCeEArlnAGLRwC%2BcAZlBCHAEQACARscKgCYCmUAHTAIAehDAYEAM7AZLANwAoZfwAekWHAF08xVPDrEAdgGMYok3ACyBAMJNIJ%2FiZgAKMIzAyAlImU4ODMIExl4DTxwVH44AF44AG04VSCgljwZXjoWTEC0ll4ZOiLslnyAXVS4KH4YYihrd3yggB5eYAA3AD4WtL62yig4fjwzAAs4hEjo%2FlpetMWApaX3KX4QfzjuuGaVldawbta0DDgTKP4prwgfIRl0GAAFPABzObh1kCmv6mOxU4wf5HAZLXygoLUUGtMRDBZpGEdHr5XwqahAA%3D%3D%3D&inputTab=FYZwHkA%3D

[samijaber]

This is a bug. Here we generate these slots: https://github.com/BuilderIO/mitosis/blob/e862056eca675887bd975bf5696c61666d0f3532/packages/core/src/generators/vue/blocks.ts#L297-L305

We are missing the slots' properties and bindings, which are generated by this function:

https://github.com/BuilderIO/mitosis/blob/e862056eca675887bd975bf5696c61666d0f3532/packages/core/src/generators/vue/blocks.ts#L378

I believe you should be able to reuse that function. If not, feel free to modify it as needed

[Marvin-Brouwer]

I'm not sure how to "reuse that function", do you mean when creating a bugfix?

[samijaber]

Yes, I am describing the process of contributing a fix to Mitosis itself 😄

[Marvin-Brouwer]

AH, check. I might take some time somewhere next week to do so.

I see react and svelte also aren't binding this, is this something you'd want? And for example for react, would this mean it would output like <p>{slotExample(value)}<p/> or would we maybe want to include a <Slot> component for react? And do we know of more components that might need an update?

[Marvin-Brouwer]

@samijaber I pulled down the repo and I immediately have failing tests: Is there anything I need to configure?

[samijaber]

There might be a problem with the configuration of the repo. Do the tests pass if you run yarn ci:build in the root?

[Marvin-Brouwer]

Yes that seems to work out fine. So running yarn test in the ~/packages/core folder doesn't work as expected.

Additionally, I now have 22 modified files after running yarn ci:build in the root. Are you on unix or mac by any chance? All of the changes appear to be whitespace.

[Marvin-Brouwer]

I added some editorconfig to lock the newlines on unix line endings and now tests don't create new updates. So, I'm sure that was it. I'll use yarn ci:build to validate my work for now and leave yarn test in the ~/packages/core folder out of scope for now.

I've done some digging and react seems to want you to use any kind of dynamic component like so:

<p>DynamicComponent({ prop: value })</p>

So I'd propose to change the slot implementation in react to something similar.

<ul>
  {items.map((item) => (
    <li>{ slotExample({ item }) }</li>
  ))}
</ul>

However, that would mean the regular slots will probably be better off looking like:

<p>{ slotRegular() }</p>

Are you okay with that?

I'll focus on vue first, then I'll look at svelte and then I'll tackle react. I'll have a look at other formats after that, but I recon I'll need some time to get the vue fix in first.

Let me know if you have any questions, concerns or other remarks.

[Marvin-Brouwer]

@samijaber I'm having a hard time understanding this part:

It looks like that in the case of a nameless slot, when there is anything in json.bindings the vue template mechanism should be used. However, these don't seem to be specified in the snapshot tests. How is this supposed to behave? And what about when someone would add a <Slot key="whatever" ... on their component? Should it also handle that key property?

I added some test scenarios to make it consistent. With my current changes it looks like this: This doesn't seem to behave consistent, and I'd like to fix that. However, I'd like to know the intended behavior before I attempt to make it consistent.

[Marvin-Brouwer]

Hi,

@samijaber I've figured some things out and made some progress but I'm kind of stuck right now. When generating the snapshots somehow there's a type definition generated:

I can't find where this happens. Could you please point me to the block of code that is responsible for generating these propType definitions?

Thanks

[samijaber]

These definitions are stored in the types key, and the name of the type (Props in this case) is stored in propsTypesRef:

https://github.com/BuilderIO/mitosis/blob/e862056eca675887bd975bf5696c61666d0f3532/packages/core/src/types/mitosis-component.ts#L148-L149

which is then consumed by each generator, like here:

https://github.com/BuilderIO/mitosis/blob/e862056eca675887bd975bf5696c61666d0f3532/packages/core/src/generators/react/generator.ts#L561

[samijaber]

If you are concerned about the slot prefix added to slots in React, that is being fixed right now in #1334

[Marvin-Brouwer]

No I'm not, I converted react slots from a field to function so data can passed just like in vue, svelte, etc.

However, the type still generates JSX.Element instead of () => JSX.Element or (slotProps) => JSX.Element So, I'd like to take a stab at rectifying that.

[samijaber]

I just looked at the snapshot you screenshotted:

https://github.com/BuilderIO/mitosis/blob/e862056eca675887bd975bf5696c61666d0f3532/packages/core/src/tests/data/blocks/content-slot-html.raw.tsx#L2-L8

It is actually an error to import the Mitosis JSX.Element here. For now, the best we can do is make this slotTesting: any, because Mitosis does not properly handle generating element types across all frameworks.

So it is safe to ignore this type or change it to any in the .raw.tsx file if you prefer.

[Marvin-Brouwer]

Ah, check. I see now. I was convinced these types were generated for React since it's the only framework that doesn't have some kind of slot mechanic. But this makes sense to me.

I'll continue with solid then.

I may have some backwards compatibility questions when I'm done. But I'll first see how far I can get this to work across all output types.

[samijaber]

@Marvin-Brouwer I just re-read your implementation suggestion and realized it could cause breaking issues for other folks.

Would you mind firing up a draft PR with what you have so far, so I can see what you mean exactly and align on the implementation? This will prevent you from wasting time in a direction we don't want Mitosis to go into.

[Marvin-Brouwer]

Sure, on it!

[Marvin-Brouwer]

Here you go: https://github.com/BuilderIO/mitosis/pull/1336 I included some comments about the backwards compatibility thing I'd like to address if you decide to go forward with this.

[Marvin-Brouwer]

I actually noticed that solid already has this feature. BTW, if this is not the direction you'd like to go on. Could this also be implemented as a plugin perhaps? I really needs slots with data for what I'm doing

[Marvin-Brouwer]

@samijaber Any news?

[Marvin-Brouwer]

@samijaber is there anything I can do to help move this forward?