Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I can't migrate my component to Svelte 5 #15125

Open
Enes5519 opened this issue Jan 28, 2025 · 8 comments · May be fixed by #15148
Open

I can't migrate my component to Svelte 5 #15125

Enes5519 opened this issue Jan 28, 2025 · 8 comments · May be fixed by #15148
Labels

Comments

@Enes5519
Copy link

Discussed in #15117

Originally posted by Enes5519 January 27, 2025
The svelte 4 component I will share below is the wrapper version of a custom element from a different library on the Svelte side. The code below was working perfectly with the use of slots, but now I cannot achieve the same perfection because the slot rules are very strict. At the first stage, I was going to update it only by upgrading the version, can you help me at this stage?

<script lang="ts">
  import type { BlDialog } from "@trendyol/baklava";
  import { onDestroy } from "svelte";

  interface $$Props extends Partial<BlDialog> {
    class?: string;
    "data-testid"?: string;
    fullWidthAction?: boolean;
  }

  export let open = false;
  export let fullWidthAction = false;

  function handleDialogOpen() {
    open = true;
  }

  function handleDialogClose() {
    open = false;
  }

  onDestroy(() => {
    // @see https://github.com/Trendyol/baklava/issues/778
    document.body.style.overflow = "auto";
  });
</script>

<bl-dialog
  on:bl-dialog-open={handleDialogOpen}
  on:bl-dialog-open
  on:bl-dialog-close={handleDialogClose}
  on:bl-dialog-close
  on:bl-dialog-request-close
  class:full={fullWidthAction}
  {open}
  polyfilled={true}
  {...$$restProps}
>
  <slot />
  <slot name="primary-action" slot="primary-action" />
  <slot name="secondary-action" slot="secondary-action" />
  <slot name="tertiary-action" slot="tertiary-action" />
</bl-dialog>

<style>
  .full bl-button {
    --bl-button-display: block;
    flex: 1;
  }
</style>

Normally I translated it as below, but do I need to create a separate prop for each component to pass the slot attribute? For example, I made the bl-button custom element a component and I cannot use them under each other.

<script lang="ts">
  import type { BlDialog } from "@trendyol/baklava";
  import { onDestroy, type Snippet } from "svelte";

  interface Props extends Omit<Partial<BlDialog>, "children"> {
    "data-testid"?: string;
    fullWidthAction?: boolean;
    children?: Snippet;
    onOpen?: (e: CustomEvent<{ isOpen: boolean }>) => void;
    onClose?: (e: CustomEvent<{ isOpen: boolean }>) => void;
    onRequestClose?: (
      e: CustomEvent<{
        source: "close-button" | "keyboard" | "backdrop";
      }>
    ) => void;
  }

  let {
    open = $bindable(false),
    fullWidthAction = false,
    children,
    onOpen,
    onClose,
    onRequestClose,
    ...rest
  }: Props = $props();

  function handleDialogOpen(e: CustomEvent<{ isOpen: boolean }>) {
    open = true;
    onOpen?.(e);
  }

  function handleDialogClose(e: CustomEvent<{ isOpen: boolean }>) {
    open = false;
    onClose?.(e);
  }

  onDestroy(() => {
    // @see https://github.com/Trendyol/baklava/issues/778
    document.body.style.overflow = "auto";
  });
</script>

<bl-dialog
  onbl-dialog-open={handleDialogOpen}
  onbl-dialog-close={handleDialogClose}
  onbl-dialog-request-close={onRequestClose}
  class:full={fullWidthAction}
  {open}
  polyfilled={true}
  {...rest}
>
  {@render children?.()}
</bl-dialog>

<style>
  .full bl-button {
    --bl-button-display: block;
    flex: 1;
  }
</style>

Problematic usage

<DialogV5 bind:open {onRequestClose} {...rest}>
  <div class="container">
    <AssetImage name="{variant}-illustration.svg" alt={variant} />
    <div class="caption">{caption}</div>
    <div class="description">
      {@render children?.()}
    </div>
  </div>

  {#if primaryButtonText}
    <Button slot="primary-action" class="button" size="large" onClick={onClickPrimary}>
      {primaryButtonText}
    </Button>
  {/if}
  {#if secondaryButtonText}
    <Button slot="secondary-action" class="button" size="large" variant="secondary" onClick={onClickSecondary}>
      {secondaryButtonText}
    </Button>
  {/if}
</DialogV5>
```</div>
@Enes5519
Copy link
Author

I created a workaround. I create a prop called slott and set it with the actions. But I didn't like this solution very much.

@paoloricciuti
Copy link
Member

Sorry I've read this multiple times but I don't get what's the issue you are facing. What is happening? Why can't you just create a prop for each snippet?

@paoloricciuti
Copy link
Member

Maybe the problem is that you are using the slot property within the snippet?

@brunnerh
Copy link
Member

@paoloricciuti check the linked discussion. The problem is an interaction with custom elements where Svelte prohibits the use of slot="..." (which previously was not the case).

@paoloricciuti
Copy link
Member

@paoloricciuti check the linked discussion. The problem is an interaction with custom elements where Svelte prohibits the use of slot="..." (which previously was not the case).

Oh yeah that was my idea too...mmm this is a tricky one

@Enes5519
Copy link
Author

We are having trouble with sourcemaps and want to migrate to Svelte 5. This prevents us from making an easy transition. How do you think we can proceed?

@paoloricciuti
Copy link
Member

An easy workaround would be to use the snippet to only populate the internal of the element. So basically instead of @render children() you do

<button slot="blabla">
    {@render children()}
</button>

But that might not be fully possible.

@Enes5519
Copy link
Author

If I make such a move, it means that I will use the action slots only for the button, which will create a big breaking change in our project. Additionally, each button has its own variants. Unfortunately, I cannot change the Dialog component in this way.

dummdidumm added a commit that referenced this issue Jan 30, 2025
`slot` is treated as a regular prop if it is not used directly inside another component, but we were running validations on such regular props that should only be run on real slots.

Fixes #15125
@dummdidumm dummdidumm added the bug label Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants