Conversation
|
@mustafa0x fixed the one I thought it was worth fixing and were not false positives (btw thanks for the review)...commented on the rest just for the review process in case someone disagree with me: 18, 19: I don’t think that’s an issue…an error on those paths would still break the app I think 20: I don’t think it’s an issue wither…it would actually be wrong the opposite imho, converting everything to string works for the DOM not necessarily for custom renderers 23: not a huge issue imho…it will still fail to import the renderer so if you have an empty string you have much bigger problems 25: not an issue: the return type is specifically ‘’ to avoid going through any specific DOM path 40: I don’t think it’s relevant 8: 13: I wonder if this should be fixed… I can see how someone might want to create a render that mimic the DOM one, but other than “for fun” what’s the reason? 24: let’s be honest…if you fuck up your import it’s up to you to correct it lol 27: not sure is worth the complexity…nobody is gonna generate server AND custom renderer 25: that’s fine, it’s dev only and that’s why it needs to be an object of some sort 36: left a to-do, but probably not worth fixing now 37: not really an issue imho…migrate is basically dead code now 38: null should be still treated as false Priority 2 and 3 are all out of scope imho |
|
does attachShadow need to be in the interface? for |
|
IMHO |
|
it's just a bit goofy, because you don't actually emit a |
|
The problem is that |
|
Small update: Now you can technically interleave components compiled with different renderers (imagine a DOM component into a Threlte one) but:
|
38d45c1 to
4440cec
Compare
|
AI finally wrapped up it's review. It claims ~30 P0s remain. That number seems high, and most of the issues seem a bit obscure. Note: I incorporated your fixes and your feedback into the review. I hope the review is of some use, and not only noise. |
|
Is absolutely of use and not at all noice, especially done in a separate file like this...will take a look later/tomorrow/Tuesday (depending on when I will have some time). Thanks 🤟🏻 |
|
Checked them...a lot were wrong assumptions but overall solid ideas
|
Closes #15470
We're so back!
Finally, thanks to Syntax and SuppCo that sponsored the Custom Renderers initiative, I was able to work full-time for a bit on this (thanks again to my employer, Mainmatter for allowing me to do this).
This, the fact that we recently revisited the direction we were going (that lead to much less code needed), plus the fact that Claude now is decently good at navigating the Svelte codebase allowed me to do a lot of progress (I was also able to re-use part of the code I already had before).
As you can see, this is a big PR, but I tried to split the commits reasonably so that the review process shouldn't be too bad and there wasn't really a way to verify it worked before having built most of it. There are still a few To-dos but luckily, I also have a few more days to work on this (and at this point is in a place where I can also do it in my spare time).
To-dos
How it works
experimental.customRendereris a new compile configuration option. It can be astringor a function that accepts the filename and returns astring. The value should be an NPM package or a module that exports the renderer as its default export. When this option is defined, the compilation output changes a bit (no delegated events, no inlining ofnode.nodeValue="", no customizable select, etc...basically we're doing a lot of optimization that are specific to the DOM which are skipped).The compilation also changes because now the compiled component imports the module you specify, uses
from_treeinstead offrom_html, push the renderer at the beginning of the component and pop it at the end...basically thisgets compiled to
What does a custom renderer looks like? You can have a look at the one I created for testing in
packages/svelte/tests/custom-renderers/renderer.jsto have a more practical example but basically, you can importcreateRendererfromsvelte/rendererand then specify a series of DOM-like operations in your "world".You can then use the return value to "mount" your component
A good custom renderer is crucial to make svelte works properly so we will need to document this correctly (even though I don't expect people to create custom renderers in their day to day and the Svelte team will likely be the primary user of this API).
A few examples of this:
insertassumes that the insertion works like the DOM: if you insert something that already has a parent it should be removed from where it is.insert-ing a fragment means inserting all the children of the fragment in the parent.Limits
A few features of Svelte are designed specifically for the DOM and thus are disabled if you try to compile a component with a custom renderer:
bind:on regular elements is forbidden, since svelte register known DOM events to keep the variables in sync.transition:,animate:,in:andout:are forbidden, since, once again, those use DOM manipulation under the hood.svelte:window,svelte:body,svelte:document,svelte:head... I mean, do I really need to explain why this is forbidden?css: injectedis also forbidden since it appends thestyletag to the documentcreateRawSnippetthrows a runtime error since it relies on thetemplatetag to generate the HTML elements from the string you returnAnother quirk is that you can technically interleave components compiled with different renderers (imagine a DOM component into a Threlte one) but:
beforefunction on the comment that will receive the fragment/element/text that the component is trying to "mount")@rendera snippet compiled with a different renderer from the one that is invoking@render. This means if I'm mounting a DOM component into a Threlte component I can't pass a snippet (unless that snippet is exported from a component compiled without a renderer and imported into the Threlte component)These limitations might change before we merge if we find a way to make them work.
What this PR does?
The main job of this PR is to centralize every DOM operation in
operations.jsas an exported function. This allows the function to check if arendereris available so that it can call the method on the renderer instead of the DOM method. The renderer is also captured in every effect created in the component, since it needs to be "pushed" again before the effect execute. The same is true for boundaries, batches and each blocks.I've also added a somewhat decent test suite that uses a render-to-object renderer that renders the svelte components to...well...an object. This allows the tests to be "similar" to the rest of the test suite (there's even an object-to-HTML string helper to assert the shape of the component) but is executed in a node environment, so every access to DOM API will actually throw.
I've changed some of the validation in place, but there's still a few warnings and errors that don't make sense, which I plan to fix before merging.
A few questions
objectcould help us with the maintainability of the custom renderers API (now Typescript will yell at us if we try to accesselement.valuewithout checking)...but it could make the maintainability of DOM Svelte a nightmare (because now you have to check everything and everywhere). Should we keep it a lie?Extra
To test this out, I (admittedly Claude) built an opentui custom renderer to render svelte component to the terminal...here's a small preview.
Screen.Recording.2026-03-31.at.15.17.13.mp4