Feat(solid): Handle anchor node and add Dynamic and Portal#169
Feat(solid): Handle anchor node and add Dynamic and Portal#169
Conversation
|
|
|
|
||
| children = testSetup.renderer.root.getChildren()[0]!.getChildren() | ||
| expect(children.length).toBe(1) | ||
| expect(children.length).toBe(2) |
There was a problem hiding this comment.
Solid subs existing element with the empty node so number of children remain same
| @@ -0,0 +1,130 @@ | |||
| import { BaseRenderable, isTextNodeRenderable, TextNodeRenderable, TextRenderable } from ".." | |||
There was a problem hiding this comment.
Are the slots something reconciler specific that we could have in the solid package? For core itself they don't seem to have a use-case.
There was a problem hiding this comment.
Yeah will have to explore around the api to make it work well for core. But I was thinking if it had a replaceSelf method, which allows us to create a renderable with slots and expose those such that on usage we could just slot in custom renderables. For eg Dialog component with header, content and footer slots.
This is sort of a low cost node that can be replaced when needed.
I will move it for now tho.
| await testSetup.renderOnce() | ||
|
|
||
| setComponentType("box") | ||
| await testSetup.renderOnce() |
There was a problem hiding this comment.
Is this missing an expectation or just testing that it doesn't crash?
|
|
||
| it("should throw on rendering text without parent <text> element", async () => { | ||
| expect( | ||
| testRender(() => <box>This text is not wrapped in a text element</box>, { |
There was a problem hiding this comment.
I hope we can get this to work in the future, would be so neat.
kommander
left a comment
There was a problem hiding this comment.
I would move the Slot renderables to the solid package, as they are reconciler specific, other than that looks great 👍
- Simple `bun test` should also work now - Added tests for Dynamic and Portal - Brought in universal renderer for custom addAnchorNode logic - Fixes a bunch of bugs around Show and For - We throw on not adding unwrapped text/span.
…eplacement The forked universal renderer diverged from solid-js's upstream behavior in cleanChildren(). When called without a marker or replacement (e.g. when Show/Switch hides content), it returned `replacement` which is `undefined`, instead of returning "" like solid-js does. This caused undefined to propagate through the reactive graph as the `current` value in insertExpression, eventually corrupting downstream memo reads. Introduced in #169 when the universal renderer was forked.
…eplacement (#670) ## Summary - Fix `cleanChildren` in the forked universal renderer to return `""` instead of `undefined` when called without a replacement, matching solid-js's upstream behavior ## Problem When `cleanChildren` is called without a `marker` or `replacement` (e.g. when `Show`/`Switch` hides content), the no-marker early-return path returns `replacement` — which is `undefined`. The solid-js upstream returns `""`. This `undefined` propagates as the `current` value through `insertExpression`'s reactive chain, eventually corrupting downstream memo reads and causing crashes like: ``` TypeError: undefined is not an object (evaluating 'grouped().length') ``` Introduced in #169 when the universal renderer was forked from solid-js. ## Fix ```diff - return replacement + return replacement ?? "" ```
bun testshould also work now