Functional children of suspense never resolve

[Tanner-Scadden]

Describe the bug

If children of suspense is a function they never resolve. Here are some links with a weird fix for it that renders the child but not the fallback, and one where it fixes both. Also a link to the discord

Renders child but not the fallback: https://playground.solidjs.com/anonymous/75137e0b-da75-46d0-8faa-08d8cff596a4

Renders both but required the children to be invoked https://playground.solidjs.com/anonymous/76380071-8fb7-48af-898a-7037005658bf

More in depth playground example showing that wrapping the children of the Suspense with a wrapper fixes it, but then when the child falls into suspense it bubbles up to the parent: https://playground.solidjs.com/anonymous/290e63e2-7873-4a86-8770-cad66bf47ab3

Link to discord where the conversation started with multiple reproductions and ideas: https://discord.com/channels/722131463138705510/722131463889223772/1073714796710604961

Your Example Website or App

https://playground.solidjs.com/anonymous/a288d594-f9ee-4ddd-ba8c-c85ca8028ad1

Steps to Reproduce the Bug or Issue

1. Have a reusable Suspense Component
2. Pass a function into the SuspenseWrapper as the child
3. Promises are never resolved and it stays in the fallback.

Expected behavior

As a user, I expect to be able to make a reusable suspense wrapper that can accept children as a function and that suspense is able to display fallback's correctly and resolve.

Screenshots or Videos

No response

Platform

• OS: [e.g. macOS, Windows, Linux]
• Browser: [e.g. Chrome, Safari, Firefox]
• Version: [e.g. 91.1]

Additional context

This is for the error boundary ticket, but wrapping the children with the utility children function seems to resolve this issue as well. https://playground.solidjs.com/anonymous/e787c74b-cbee-427f-9ac0-b845bec1e6a7

No response

[ryansolid]

I think these issues are by design. Function children aren't suppose to do anything different. If you pass something a function and it returns a function that function isn't called by it.

Like consider:

const a = () => console.log("a");

const b = () => a

Calling b, doesn't execute a simply returns it. Suspense or Error Boundaries should never have function children. It's nonsensical. JSX types don't prevent it. But it is passing straight through. b is your Suspense Component here.

Eventually when you get to the DOM it needs to unwrap all the functions. But not every component is going to do that.

I do think something internal to Suspense is messing up here hence the error. It does unwrap internally I believe. I generally don't entertain this scenario because it is basically invalid. Like would you pass a function to a random component and expect it to work when it isn't expecting one?

[thetarnav]

Not sure if this is intended or not, but I would avoid passing function children to any control flow component (if the types clearly do not expect it). When function is necessary for creating variables in JSX, I would wrap it in untrack so that it's called immediately when props.children are accessed.

[fabiospampinato]

Eventually when you get to the DOM it needs to unwrap all the functions. But not every component is going to do that.

I generally don't entertain this scenario because it is basically invalid.

This explanation doesn't really resonate with me at all.

One core aspect of how reactivity systems work, maybe the main one, is that children are executed inside their parents, if that's not happening the whole thing falls apart.

In Solid we can pass arbitrarily-wrapped children like that kind of everywhere (https://playground.solidjs.com/anonymous/801252a1-734b-413e-86b7-4603cc630512), and they are resolved, because what else is the framework supposed to do with children functions? If children are resolved, but they are not resolved inside their parents, then it just looks like a bug to me.

For context it works like this in Voby (https://codesandbox.io/s/voby-playground-forked-141k8h?file=/src/index.tsx), it doesn't matter how many times a child is wrapped, whatever the child is the framework will resolve it, or it will just be broken.

I don't see why this would work any differently in Solid, it looks to me like children are being resolved, but they are being resolved at the wrong time, sometimes.

I can't think of a scenario where Solid's behavior in this regard leads to more correct output than Voby's, if anybody has one I'd like to see it. Like resolving children inside their parents seems just fundamental to me.

Like would you pass a function to a random component and expect it to work when it isn't expecting one?

What's a component that doesn't expect (also) functions as children? Solid's own JSX types are telling me that passing functions as children like in @Tanner-Scadden's example is valid. Like by definition Solid is telling me that passing a function there is ok.

[Tanner-Scadden]

Why does it execute correctly then if there is a div around the child components?

I'm not sure why the ErrorBoundary ticket #1543 was closed. It has this same issue, and if you wrap the child that's a function in a wrapper or use the children helper, it catches the error correctly.

Not being able to pass functions as children as props makes me very confused on how things actually work. Why does fallback on Show, Switch, Suspense, and ErrorBoundary accept functions? Wouldn't that be invalid?

What's the difference when it's executed between passing it in as a function and rendering it directly? From my knowledge, it's no different than this example:

<input onChange={handleChange} /> vs <input onChange={e => handleChange(e)}.

All we have added from the child being a function that returns a component is an extra step. If solid followed the rules of your console.log example, the components wouldn't be rendering.

While I'll try to avoid this pattern in the future if it's not okay, if typescript tells users that this is okay, how do I prevent people from doing this in a reusable library?

[ryansolid]

The only reason this one is open is because it shouldn't get in an infinite loop. Either it should work or it should not trigger Suspense. But the fact it would work is an implementation detail, because the usage is invalid. So I was tempted to close this one as well.

Passing a function to a child that doesn't expect it doesn't make sense. It's more exploiting a necessary part of the insert mechanism. It is completely different to passing to something that intends a callback. Forget reactivity for a moment or any expectations there and just consider if you were writing code that expects a value and you pass it a function.

Example:

const sum = (a, b) => a + b;

const getNumber = () => 2;

sum(getNumber(), 3) // 5

// would you expect this to work?
sum(getNumber, 3) // 3

Technically they both produce a sum the first is correct and proper usage and gets the right answer. The second works as a necessity of the JavaScript engine but is incorrect usage and produces the wrong sum.

You can think of the control flow as a builder function. It doesn't really care what you pass as the consequence unless it explicitly supports callbacks (like some do). It gets an input and builds it into the right structure to be inserted. You pass invalid input in, you get invalid input out. It might sort of work but that is besides the point.

It'd be interesting to see if we could make TypeScript smarter here but I kind of doubt it can be. We all know TypeScript with JSX is flawed. Maybe there is a way we can use that to our advantage? If it views all component returns as JSX.Element can we tell it functions aren't accepted as JSX.Element and force authors of those components to cast?

The thing is ultimately this expectation of passing an arbitrary function to a Component who has its own logic on handling its props and isn't expecting a function is an incorrect expectation. @fabiospampinato Voby might work this way because it needs to handle the might be function case always. We don't by design. If TS worked properly this would be very clear.

[fabiospampinato]

Ok so if I'm hearing you correctly your position is basically that Solid is using the wrong language, like it's TypeScript's types that are wrong, but they can't be fixed, because TypeScript doesn't allow for that.

Your example with the sum function makes sense to me when viewed through the lens of TS (I see JS as a compilation target at this point), that code wouldn't make sense with TS, because addition isn't defined between functions and numbers, and I'll get an error about it, as expected.

If I'm understanding your reasoning then a similar situation is happening with Solid's components, except that the types that you want to support cannot be written, so what components say they support isn't always actually true, but TS doesn't give the user an error about it.

I still don't understand why components shouldn't always resolve their children though, in your sum example summing a function and a number is undefined so it doesn't make sense, but in this example putting a function inside a parent component could basically always be well defined, as proof of that it just works like that in Voby. But you are saying Solid can't or doesn't want to do that for reasons that I don't entirely understand.

I'd suggest revisiting this decision first of all, and alternatively checking for types at runtime and throwing with a clear error when something doesn't match, and making your case for changing how JSX works in TypeScript in TypeScript's repo.

That's pretty much all I have to add to the discussion I think. This behavior doesn't make sense to me, but it is what it is.

[Tanner-Scadden]

I can understand your logic, and in those cases it makes sense. However, in the cases where the child is not expecting any props, it is just returning a component that is already invoked, I am lost on why it wouldn't work.

Playing around with it more, when I wrap these functional children in my reusable suspense or error boundary component like this:

<Suspense fallback={() => <span>Loading...</span>}>
   {untrack(children(() => props.children))}
</Suspense>

It works how I would expect it too. Suspense boundaries are respected and ErrorBoundary catches errors instead of bubbling up.

I'll be adding these wrappers for the children until there is a way to easily warn the user that what they are doing is invalid with resources to understanding why, or the reasoning behind this is rethought. Either way, appreciate your time and enjoy your birthday weekend!

[ryansolid]

If you wrote a JavaScript library that did a sum and then for some reason TypeScript couldn't type it properly would you blame the library or TypeScript? You'd just think well, I guess TypeScript isn't that useful. And then everyone insists on using it and you are like, alright um sure. I'm very open to anyone helping with the types here.

The reason I'm fixing in on the API is that with Solid 2.0 in the works, yes the implementation will change. Lazy Memos will make it that our mechanisms that rely on eager memos will need to change. This is a small part of end user code but it affects rendering and control flow internals will need a bit of an adjustment. This case might even work unintentionally. That being said what I've said above from an API standpoint is unchanged by that. An event handler expects a callback so you pass it that. A sum expects a number. And an ErrorBoundary/Suspense expects reactive expression or < /> element children.

Honestly, I do wonder if this is fixable with the types. I viewed TypeScript early on the way I viewed typed languages like C#. The truth is sometimes with TS it is better to lie for API surface reasons when dealing with things where DSLs can't perfectly be typed. Things like Proxies and JSX make this much more apparent. End users will get clear and usable types that guide them properly and the internals can contain all the messiness of JavaScript.

[ryansolid]

I can understand your logic, and in those cases it makes sense. However, in the cases where the child is not expecting any props, it is just returning a component that is already invoked, I am lost on why it wouldn't work.

Playing around with it more, when I wrap these functional children in my reusable suspense or error boundary component like this:

<Suspense fallback={() => <span>Loading...</span>}>
   {untrack(children(() => props.children))}
</Suspense>

It works how I would expect it too. Suspense boundaries are respected and ErrorBoundary catches errors instead of bubbling up.

I'll be adding these wrappers for the children until there is a way to easily warn the user that what they are doing is invalid with resources to understanding why, or the reasoning behind this is rethought. Either way, appreciate your time and enjoy your birthday weekend!

Why would this be necessary? What component that is already invoked is returning a function? Like I'm missing why this whole thing is even a thing. We recommend returning Fragments or Memo's if you must return something dynamic.

Where does this happen? Again why would one expect this to work? The problem is if you start doing this and people use the library then they might have incorrect expectations as well. I'd be interested to see the situation where this came up and determine if there is an easier way to address the confusion upfront.

[fabiospampinato]

If you wrote a JavaScript library that did a sum and then for some reason TypeScript couldn't type it properly would you blame the library or TypeScript? You'd just think well, I guess TypeScript isn't that useful. And then everyone insists on using it and you are like, alright um sure. I'm very open to anyone helping with the types here.

I'd be 100% on your same boat in that case. I just see this as a different case, not the same.

Maybe you are even right, I don't entirely understand your reasoning for wanting Solid to work like this after all, but if the resulting code is not understandable by the end user, and if writing a better programming language is out of the question, then bending what you want to satisfy the language you are using may be the best option.

Like as an hypothetical user, in general, unless I'm forced to use a program I won't use it if a) I have no clue about how it works, or b) I understand how it works, and I disagree with it. In this particular example I don't think Solid has cleared that bar for me for example, I don't 100% understand how it works, and I kinda disagree that it should work like that.

Though one can't make everybody happy of course, just saying.

[ryansolid]

Serious question though.. can we make the types stricter here? What would be the implications. @otonashixav @trusktr.

Like from a JSX standpoint syntactually we never need to pass a function in.

// Show returns a Memo but it could be cast to JSX.Element

<div>
  <Show /> // thinks it is JSX.Element
</div>

<div>{readSignal()}</div> // valid
<div>{readSignal}</div> // invalid, technically works but would error for TS

From my standpoint I don't think anyone should consider this valid:

<div>{() => <span />}</div> // since when do DOM nodes take functions? Would React let you do this?

From my point of view this is all quite consistent, but TS tells us we can do more than it should.

[Tanner-Scadden]

I tried it in react and it does not throw a typescript error, but there is an error thrown inside of react to give feedback to the user.

Here is a playground example: https://playcode.io/1176221

[otonashixav]

I haven't read the discussion completely, but if you're asking if we could remove functions from JSX.Element and simply cast the return of Show and gang with as unknown as JSX.Element, I personally don't see any issues other than breaking code for people currently returning memos. children might need to be typed differently. Also manually unwrapping props.children might be more confusing.

One interesting alternative is attaching a fake property to denote memos, without changing anything at runtime, so that functions would not be usable in TS JSX (without casting), but memos could be typed with the property and be usable.

// dom-expressions

// non-existent symbol
declare const MEMO: unique symbol;
interface FunctionElement {
  (): JSX.Element;
  [MEMO]: never;
}

// solid
type Memo<T> = (() => T) & { [MEMO]: never };
function createMemo(...) { 
  //...
  return accessor as Memo<T>;
}

function Show(...) {
  //...
  return createMemo(...); // totally okay

I'm sure this is something most people would be against. It is fun though.

[ryansolid]

Oh that is interesting. I mean adding it to the Accessor type would be sufficient. Memos and Signals are fine in these cases. Syntactually I'd almost prefer to be strict on all functions but this might ease that migration. Only downside I can see is people opting to overwrapping but at least it would flag that something is wrong. I am very tempted to do this. Maybe we just found a reason to push to a 1.7 release.