isPending(..., true) is inconsistent with behaviour of Loading boundaries

[GabbeV]

isPending(..., true) differentiate between the source being initially pending or pending for this update. Loading also had this behavior before but that got changed/fixed as response to this issue: #2672.

What phase something is in should be based on the read instead of the source if these things are to be consistent.

This has been discussed on discord here: https://discord.com/channels/722131463138705510/780502110772658196/1504500656394997902

The fix would produce similar behavior to this:

function newIsPending(fn, loading) {
  if (loading) {
    fn(); // potentially try catch errors that aren't not ready errors here
  }
  return isPending(fn);
}

However reading fn twice would be nice to avoid if possible.

Here is a demo how the current isPending compared to this changed behaviour and how it composes with Loading boundaries.
https://s.olid.uk/id/kpIvSAwkQt6lfTbMUC6Q8g

[ryansolid]

I think the behavior described here is the expected behavior semantically, but the tricky part is whether we can detect it without changing what isPending means.

isPending generally acts as an optimistic/status read. It is intentionally used to guard against normal pending throws so existing UI can stay live while newer async work is pending. So isPending(fn, true) cannot simply mean “perform a normal read first, then check pending”; most of the time it still needs to behave exactly like normal isPending(fn).
The true flag is more specifically about participating in Loading/top-level not-ready handling. It should let a not-ready read propagate when there is no committed UI at the consuming parent to preserve. But the naive implementation shape:

if (loading) fn();
return isPending(fn);

is too general because that first fn() is a normal read. If anything inside fn is pending, it will take the normal pending throw path before isPending has a chance to turn the read into an optimistic/status check. That means isPending(fn, true) would effectively always throw for pending reads, including revalidation cases where isPending is supposed to keep existing UI alive and just report that stale data is being shown.

For example, during revalidation an async source may be pending while still having a previously committed value. In that case isPending is the mechanism that lets the UI keep rendering the old value and show an inline pending state. The double-read shape would throw on the first normal read and turn those optimistic paths back into Loading/top-level pending behavior, defeating the purpose of isPending.

So the rule cannot be “loading mode starts with a normal read.” It needs to be “loading mode behaves like normal isPending unless this read is part of a not-yet-completed parent that should participate in Loading/top-level handling.”

The implementable condition may be something like:

isPending(fn, true)
should behave like normal isPending(fn) except when a pending source is read from an uninitialized parent context. In that case it should propagate the pending error so a Loading boundary, or top-level no-boundary handling, can own it.

That is different from checking only whether the source itself is initially pending. The source may already have a previous value, but the consuming parent may be new. From the consuming parent’s perspective, this is still an initial load.

There are edge cases, for example creating a new owner under an already initialized , but I think this may be the practical boundary: existing initialized parents keep optimistic isPending behavior, while uninitialized parents can participate in Loading. The problem is that optimistic updates would be bailed out of for reads of existing async on these branches. They'd get locked up in the Transition.

[mizulu]

to simplify for future readers

isPending(asyncSignal) in solid 2.x does not return true on the initial render
so if there is a ui element you want to disable while something is loading
this one is not your friend.

isPending(asyncSignal,true) can theoretically be used to detect that the signal is actually pending in its initial render
but it also throws, so it must be wrapped in a loading boundary ( See point C )

A

// This does not show pending, for the initial loading. even if `user` is pending it will be false.
Comp {
   return <>
    {isPending(user)? "pending": ""}
 </>
}

B

// This will throw  (because of the true flag)
// but because it throws it can be used to build a sync helper to detect actual pending status of the deps 
Comp {
 
   return <>
   <button disabled={     isPending(user,true)  }></button>

   <Loading>{user.id}</Loading>
 </>
}

C

So how do we show pending state for initial and for following pending status changes?

we have to use isReallyPending

  function isReallyPending(deps: () => any): any {
    return createLoadingBoundary(() => {
      deps()
      return isPending(deps, false)
    }, () => {
      return true // fallback of pending so return true
    });
  }

const shouldDisable = isReallyPending(user)
Comp {
 
   return <>
   <button disabled={     shouldDisable ()  }></button>

   <Loading>{user.id}</Loading>
 </>
}

working demo
https://s.olid.uk/id/LDzYOGqvTd-ouHTDqyuNIQ

msedge_8EE26NwH3G.mp4

D

no one knows why isPending(deps) does not return true for the initial
or it just might be too complicate for me to understand.

[brenelz]

So I thought through this a little bit. I think this is coming from a bit of a misunderstanding of expectations.

I think Solid's current meaning for isPending is narrower: isPending(fn) means “am I currently showing a stale committed value while newer async work is pending?” Under that definition, initial load returns false because there is no committed value yet.

I am thinking maybe a rename from isPending to isStale might be better? I think that removes the confusion for if initial load is included

[GabbeV]

I'm unsure what about the double read version you are saying wouldn't work as it does work in the demo. I assume it is throwing and then rerunning with the committed data finding the isPending anyway maybe? However that doesn't really matter as avoiding double read is preferable anyway.

The described functionality of basing it on the parent being new or not matches my understanding of how it should work perfectly. A Show revealing a isPending(foo, true) should suspend the update until foo is ready. This matches the behavior as if foo was rendered, it wouldn't show stale content and then suddenly swap later. Not sure what case you mean this logic would get wrong.

I do want to try sell you on the idea of potentially making this the default or even only behavior of isPending once more though.

I believe Loading boundaries being the only way to cut the dependency on downstream things keeps the mental model simpler. You either depend on a value or not. No ghost dependencies that observe things but don't depend on them. The consistency between reading foo and checking isPending(foo) makes things like using foo in an event handler guarded by isPending(foo) just work out naturally. I do think we want to avoid having to tell people to add dummy dependencies as much as possible.

Current isPending(foo) can be expressed using newIsPending(() => loading(foo, () => null)) and the version of isPending zulu is after can be expressed as loading(() => newIsPending(foo), () => true) (loading and newIsPending as defined in my linked demo). This to me says that the current isPending is mixing up loading and pending semantics into one primitive.

Do you have any example of this "guard against normal pending throws" pattern that could demonstrate why I'm wrong here?

[ryansolid]

It'd probably be fine in the demo but not in general. I've been thinking about this more and I might have a simpler way to describe the problem.

The whole system works on decoupling ownership from update. The intersection is through render effects. It is only through a final sink that we know how boundaries are impacted. Reactivity is simple it throws or doesn't throw. Eventually an effect catches that and looks up to a boundary. If it doesn't find an active one it holds as a transition. The commonality is because the source throws we can't complete the computation. In absence of that we bail out or show the previously calculated value.

isPending generally doesn't throw. This means downstream calculations can complete. They can see true or false. If it throws the consumer only has 2 options throw itself, or show previous value (false). So isPending throwing breaks optimism. Reading from the underlying value when async is present will always throw making it pointless. It could never show true.

So the first thought is scope it. Only throw under certain conditions. This gets us to the problem. No part of the reactive graph is guarenteed to know where its final sink/sinks are at the time of creation. You might think well we could if we were lazy, but that doesn't help us. Put a node in between multiple sinks. Each might want different answers but they are reading from the same source. And the only point that can make the decision of value or throw is where the isPending read is. There is only one answer what do you choose.

This is a long winded way of saying the only criteria in which we can decide how isPending behaves most be based on the contents of its function. Not where it will eventually be consumed. Loading can't have impact on whether it throws or not, because it could be consumed in both locations simultaneously. And at the time it runs there might not even be a sink yet.

What we know only is if the source has ever resolved. Which is enough to act as a guard. If it has resolved before even if a branch is new means we still have a value. A stale value so the field should see true and be disabled. But it couldn’t just throw because it wouldn't know it should. Otherwise the exact same scenario without Loading could and hold the transition unnecessarily.

Lastly, why we can't show true initially for SSR/Hydration. The reactive graph can/does exist outside boundaries. To show true means not throwing. But it also means not blocking the streamed chunk. At hydration time it is indeterminate what state it is, what state the server was when it flushed. Hydration could take completely different walks. Not to mention streaming mechanism uses element level markets so putting it in attributes won't work.

@brenelz isStale is probably the most accurate naming in hindsight. Full circle here.

[brenelz]

The other thought would be if it makes sense to have 2 seperate apis. Maybe thats just more confusing.

isStale(foo)   // current non-throwing stale status
isPending(foo) // loading-participating read-path check

I kind of see both sides but will have to give it more thought

[ryansolid]

@brenelz if we offered both I suspect isPending would be the more primary API. We need to pick a default to reduce confusion. I was leaning towards the stale behaviour but that isn't what pending means, hence the confusion and also the name isStale is likely less what they are searching docs for.

We could just reverse the default to make the naming clearer. Have the option be stale instead of loading.

isPending(fn, stale)