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

add slots option to component constructor options object #5687

Closed

Conversation

truongsinh
Copy link

@truongsinh truongsinh commented Nov 18, 2020

This is a continuation of #4296, which tackles #2588 (comment)

This commits also ensures that no extra code is added into svelte/internal. Instead, if users want to use slots API, they will import directly from svelte:

// parent.svelte
import { createSlot, slot } from 'svelte';
import MyChild from './mychild.svelte';

new MyCmp({
  slots: createSlot({
    slot(MyChild)
  })
});
<!-- mychild.svelte -->
<div>this div is in nested.svelte</div>
<span>this span is in nested.svelte</span>

Discussions

export function slot(componentClass: typeof SvelteComponent, options: SvelteSlotOptions): Element[] {
const wrapper = document.createElement('div');
new componentClass({...options, target: wrapper} as SvelteComponentOptionsPrivate) as any;
// @TODO this is a workaround until src/compiler/compile/render_dom/Block.ts is extended to expose created HTML element
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried but did not know how to modify src/compiler/compile/render_dom/Block.ts to expose created HTML element

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@truongsinh truongsinh force-pushed the tanhauhau/slots-option branch from 44a9731 to 7e65579 Compare December 8, 2020 19:39
@truongsinh truongsinh requested a review from benmccann December 9, 2020 04:24
@@ -99,7 +99,7 @@
"@rollup/plugin-json": "^4.0.1",
"@rollup/plugin-node-resolve": "^6.0.0",
"@rollup/plugin-replace": "^2.3.0",
"@rollup/plugin-sucrase": "^3.0.0",
"@rollup/plugin-sucrase": "^3.1.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you rebase against master you should be able to drop your changes to this file and package-lock.json. I submitted those as a separate PR in #5758 because that was a big enough change by itself that it took some review. Hopefully that'll make review of this one a bit easier by making this one a bit smaller

@akauppi
Copy link

akauppi commented Dec 29, 2020

createSlot, slot seems really confusing, what would be good names?

If it's not too late, here's my 2c:

createSlot may not be needed, at all. It's so simple I would think people are okay doing a .map when needed. For providing the default slot only, the case without createSlot is simpler than with it:

This could be done just with createSlots, as @lukeed does here.

I would prefer the plural name, since the object can carry information for multiple slots. If wanted, there can be a createSlot for only defining the default slot but that may be unnecessary overkill (and people can do such wraps easily, ourselves).

This would make your sample look like:

// parent.svelte
import { createSlots } from 'svelte';
import MyChild from './mychild.svelte';

new MyCmp({
  slots: createSlots({
    default: MyChild
  })
});

The values can be components, elements, or arrays thereof.


Would like to use this with SVG - and it can (I have a project doing that). The PR however currently contains divs, making it HTML specific.

@benmccann
Copy link
Member

@truongsinh this PR has a merge conflict. You will need to rebase against master

@jmsunseri
Copy link

This would help a lot when we are trying to write unit tests and we need to programmatically assign something to a slot?

@zachstence
Copy link

Any update on the status of these tickets/pull requests? Would love to see this feature added.

@PClmnt
Copy link

PClmnt commented Jan 28, 2022

Any update on this? It has pretty big ramifications for testing svelte components?

@SarcevicAntonio
Copy link
Member

SarcevicAntonio commented Apr 12, 2022

looks like this blocks testing slots with vitest and @testing-library/svelte, bummer...

@vekunz
Copy link

vekunz commented Jun 2, 2022

Are there any updates on this? This prevents us from testing our components well.

We need to create slot content to thoroughly test our components with unit tests.

@rousah
Copy link

rousah commented Aug 23, 2022

Would love for this to get worked on!

@benmccann
Copy link
Member

@truongsinh just a reminder that this PR needs a rebase

@unikitty37
Copy link

The lack of the ability to pass slots when creating a component is blocking a lot of people here: testing-library/svelte-testing-library#48 (the two workarounds listed there don't seem to work currently, at least for me — I really don't want to have to use Playwright's experimental component tests to do this!)

Please could this make it to release? It's been over two years for this PR…

@dummdidumm dummdidumm added this to the one day milestone Mar 20, 2023
@Rich-Harris
Copy link
Member

Closing Svelte 4 PRs as stale — thank you

@Rich-Harris
Copy link
Member

(note that this is addressed in Svelte 5, among other things with the createRawSnippet API)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.