Return type of `createRoot` doesn't consider errors

[thetarnav]

Describe the bug

After runWithOwner was changed to handle errors as any other effect would, its return type was changed to T | undefined as the returned value will always be undefined if the callback throws an error. The new catchError does it the same way. But createRoot doesn't, as far as typescript is concerned it will always return T, even though it behaves just like runWithOwner or catchError. Is this intentional? I know that handling T | undefined is a pain, and I find myself asserting it with ! quite often, but the lack of this indicator has made me believe that it could never return undefined.

Also, I guess that createMemo, mapArray, indexArray and others are in the same bucket, but changing all that would cause hell 🤣

Your Example Website or App

https://playground.solidjs.com/anonymous/d0665a89-4a79-4a81-b194-9916e96ac57a

[ryansolid]

Yeah I'm not qualified to answer this. I mean pretty much anything can error, what are we supposed to do. I think memo's and derived might be different cases. createRoot might be fine to have the or undefined.

@otonashixav I trust your judgement. Would love some thoughts.

[otonashixav]

For consistency I guess it should include undefined, unless it makes sense to change the runtime behaviour, i.e. throw if the function passed to createRoot throws, or return its return value if completeUpdates throws (basically always read fn outside the try block, instead of only if Updates is null).

[thetarnav]

I think the question is less about typescript itself and more about what users are supposed to do — handle that case or not?. It might be "fine" to not, as the error is likely to be handled by some higher error boundary anyway, but it seems like that decision should be made by the user in the end, and for that he needs to know that something like this might ever happen.

[ryansolid]

It is interesting because it does throw in all these cases unless you specifically have an error boundary above it. It rethrows when it doesn't find an error context. Of course that doesn't help with the current execution at all. Which will then error and on the next thing as well I suppose. However, technically speaking there already will be an error boundary above it.

This is only really problem on first run. I sort of wonder if the handling here is wrong in the sense we should be still throwing even with ErrorBoundaries.. I don't know. I have a hard time visualizing what this should look like. It feels weird to me to account for every callback that might error. I think stuff based on createMemo can be handled in a reasonable way not to force in undefined but I don't know if createRoot ever would. For now I'm fine making it the same as the other 2 with undefined sneaking in there, but I don't like this in general.