This issue is to report that the change in isFulfilled behavior done in #70806 was probably wrong.
The original behavior was that when isFulfilled returns true, the startResolution and finishResolution actions were not dispatched.
The patched behavior (currently in trunk) is that the resolution actions are dispatched, we're doing a kind of "fake" resolution. The purpose is to fix an infinite wait in resolveSelect.
This broke Jetpack behavior, as reported by @obenland in #68056 (comment). The original suspect was SlotFill, that's why the discussion is where it is, but it turned out that the problem is at a completely different place.
The broken Jetpack code was itself seriously buggy, but the report contains a very good point. We shouldn't do the "fake" resolution and modify the Redux state at all when isFulfilled is true. Why?
When isFulfilled was introduced 8 years ago in #6084, the purpose was to override the default memoization of selectors and resolvers. The mapSelectorWithResolver function does this check:
if ( hasStartedResolution( selectorName, args ) ) {
return;
}
I.e., when the resolver is already running or has run in the past, don't run another one. The selectorName and args values are used to create an unique key by which the resolution state is keyed. This is hardcoded. And the purpose of isFulfilled is to override this hardcoded memoization with a custom one.
That means that hasStartedResolution = true and isFulfilled = true should be handled exactly the same way. But currently the behavior is different. isFulfilled triggers the "fake" resolution.
The right behavior is that #70806 is mostly reverted, and that resolveSelect does its own isFulfilled check.
FYI @Mamaduka @johnhooks and @ellatrix. This might need to be a last-minute 7.0 patch.
This issue is to report that the change in
isFulfilledbehavior done in #70806 was probably wrong.The original behavior was that when
isFulfilledreturnstrue, thestartResolutionandfinishResolutionactions were not dispatched.The patched behavior (currently in
trunk) is that the resolution actions are dispatched, we're doing a kind of "fake" resolution. The purpose is to fix an infinite wait inresolveSelect.This broke Jetpack behavior, as reported by @obenland in #68056 (comment). The original suspect was
SlotFill, that's why the discussion is where it is, but it turned out that the problem is at a completely different place.The broken Jetpack code was itself seriously buggy, but the report contains a very good point. We shouldn't do the "fake" resolution and modify the Redux state at all when
isFulfilledistrue. Why?When
isFulfilledwas introduced 8 years ago in #6084, the purpose was to override the default memoization of selectors and resolvers. ThemapSelectorWithResolverfunction does this check:I.e., when the resolver is already running or has run in the past, don't run another one. The
selectorNameandargsvalues are used to create an unique key by which the resolution state is keyed. This is hardcoded. And the purpose ofisFulfilledis to override this hardcoded memoization with a custom one.That means that
hasStartedResolution = trueandisFulfilled = trueshould be handled exactly the same way. But currently the behavior is different.isFulfilledtriggers the "fake" resolution.The right behavior is that #70806 is mostly reverted, and that
resolveSelectdoes its ownisFulfilledcheck.FYI @Mamaduka @johnhooks and @ellatrix. This might need to be a last-minute 7.0 patch.