-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: add createRawSnippet API #12409
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'svelte': patch | ||
--- | ||
|
||
feat: add createRawSnippet API |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
packages/svelte/tests/runtime-runes/samples/snippet-raw-args/_config.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { flushSync } from 'svelte'; | ||
import { test } from '../../test'; | ||
|
||
export default test({ | ||
compileOptions: { | ||
dev: true // Render in dev mode to check that the validation error is not thrown | ||
}, | ||
html: `<div><div>0</div></div><button>+</button>`, | ||
|
||
test({ assert, target }) { | ||
const [b1] = target.querySelectorAll('button'); | ||
|
||
b1?.click(); | ||
flushSync(); | ||
assert.htmlEqual(target.innerHTML, `<div><div>1</div></div><button>+</button>`); | ||
} | ||
}); |
32 changes: 32 additions & 0 deletions
32
packages/svelte/tests/runtime-runes/samples/snippet-raw-args/main.svelte
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
<script> | ||
import { createRawSnippet } from 'svelte'; | ||
|
||
let count = $state(0); | ||
|
||
const snippet = createRawSnippet({ | ||
mount(count) { | ||
const div = document.createElement('div'); | ||
|
||
$effect(() => { | ||
div.textContent = count(); | ||
}); | ||
|
||
return div; | ||
}, | ||
hydrate(element, count) { | ||
|
||
$effect(() => { | ||
element.textContent = count(); | ||
}); | ||
|
||
}, | ||
render(count) { | ||
return `<div>${count}</div>`; | ||
} | ||
}); | ||
</script> | ||
|
||
<div> | ||
{@render snippet(count)} | ||
</div> | ||
<button onclick={() => count++}>+</button> |
8 changes: 8 additions & 0 deletions
8
packages/svelte/tests/runtime-runes/samples/snippet-raw/_config.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import { test } from '../../test'; | ||
|
||
export default test({ | ||
compileOptions: { | ||
dev: true // Render in dev mode to check that the validation error is not thrown | ||
}, | ||
html: `<p>hello world</p>` | ||
}); |
16 changes: 16 additions & 0 deletions
16
packages/svelte/tests/runtime-runes/samples/snippet-raw/main.svelte
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<script> | ||
import { createRawSnippet } from 'svelte'; | ||
|
||
const hello = createRawSnippet({ | ||
mount() { | ||
const p = document.createElement('p') | ||
p.textContent = 'hello world'; | ||
return p; | ||
}, | ||
render() { | ||
return '<p>hello world</p>'; | ||
} | ||
}); | ||
</script> | ||
|
||
{@render hello()} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -365,12 +365,20 @@ declare module 'svelte' { | |
export function flushSync(fn?: (() => void) | undefined): void; | ||
/** Anything except a function */ | ||
type NotFunction<T> = T extends Function ? never : T; | ||
/** | ||
* Create a snippet imperatively using mount, hyrdate and render functions. | ||
* */ | ||
export function createRawSnippet({ mount, hydrate }: { | ||
mount: (...params: any[]) => Element; | ||
hydrate?: (element: Element, ...params: any[]) => void; | ||
render: (...params: any[]) => string; | ||
}): (anchor: TemplateNode, ...params: any[]) => void; | ||
/** | ||
* Mounts a component to the given target and returns the exports and potentially the props (if compiled with `accessors: true`) of the component. | ||
* Transitions will play during the initial render unless the `intro` option is set to `false`. | ||
* | ||
* */ | ||
export function mount<Props extends Record<string, any>, Exports extends Record<string, any>>(component: ComponentType<SvelteComponent<Props>> | Component<Props, Exports, any>, options: {} extends Props ? { | ||
function mount_1<Props extends Record<string, any>, Exports extends Record<string, any>>(component: ComponentType<SvelteComponent<Props>> | Component<Props, Exports, any>, options: {} extends Props ? { | ||
target: Document | Element | ShadowRoot; | ||
anchor?: Node; | ||
props?: Props; | ||
|
@@ -389,7 +397,7 @@ declare module 'svelte' { | |
* Hydrates a component on the given target and returns the exports and potentially the props (if compiled with `accessors: true`) of the component | ||
* | ||
* */ | ||
export function hydrate<Props extends Record<string, any>, Exports extends Record<string, any>>(component: ComponentType<SvelteComponent<Props>> | Component<Props, Exports, any>, options: {} extends Props ? { | ||
function hydrate_1<Props extends Record<string, any>, Exports extends Record<string, any>>(component: ComponentType<SvelteComponent<Props>> | Component<Props, Exports, any>, options: {} extends Props ? { | ||
target: Document | Element | ShadowRoot; | ||
props?: Props; | ||
events?: Record<string, (e: any) => any>; | ||
|
@@ -450,8 +458,9 @@ declare module 'svelte' { | |
* https://svelte.dev/docs/svelte#getallcontexts | ||
* */ | ||
export function getAllContexts<T extends Map<any, any> = Map<any, any>>(): T; | ||
type TemplateNode = Text | Element | Comment; | ||
|
||
export {}; | ||
export { hydrate_1 as hydrate, mount_1 as mount }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to this PR, but weird why dts-buddy does this |
||
} | ||
|
||
declare module 'svelte/action' { | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone could render a component inside the snippet and then want to add something to the head. Should snippet in SSR mode therefore be required to return an object with html and head (for convenience returning a string could be a shorthand for returning an object with html)?
If yes, how would the equivalent on the client look like?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. I updated it to be
body
andhead
from the function instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this. You can't have
<svelte:head>
inside a{#snippet ...}
, which is the equivalent, surely?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rich-Harris You can have
<SomeComponent />
inside the snippet which has a<svelte:head>
though?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, scrap that, Astro and others would likely just use
mount
orhydrate
, right?https://github.com/withastro/astro/blob/main/packages/integrations/svelte/client-v5.js#L24
I reverted my changes back to just returning a string instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They do, but on the server the use render: https://github.com/withastro/astro/blob/main/packages/integrations/svelte/server-v5.js#L32
Also, if you're using
mount
andhydrate
, you get support for adding stuff to the head. So why wouldn't you be able to manually do that here aswell?(which then begs the question: How do you cleanup head content when the snippet is destroyed?)
Astro is currently not making use of properly having support for head content, but in theory they could add it I think? @bluwy is this in general possible in Astro for a framework to contribute to contents in the
<head>
tag?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean I removed it because it complicated this and Astro wasn't making use of it, plus after implementing it I ran into countless hydration issues, maybe related to #12399.
I'm also a bit unsure how someone might hydrate the head in their
hydrate
function being able to control the internal node logic.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With an effect, presumably?
I'm in two minds about this whole thing. It does feel like a limitation (regardless of whether Astro specifically would be able to take advantage of it), but getting it to work well would add a ton of complexity when you factor in hydration and stuff. Like, a simple thing like this...
...would presumably have to become this, if it's possible that
A
orB
contains head content (directly or indirectly):As opposed to this, if you just allow the constraint that programmatic snippets can't create head content:
So the increased complexity disadvantages users more than us. I suppose the alternative is that just expose the
payload
object......but that hardly feels like an improvement. So I think I land on where we are now — keeping it simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Also, there's already a limitation with this API, insofar as you have to have a top-level element. So it's not like we can advertise it as a full-blown alternative to
{#snippet}
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment I don't think it's possible, especially with regards to streaming, it's not easy to aggregate them beforehand unless Astro do something to detect if components exposes the head, which is tricky. I think it's ok to punt this off for now though.