Skip to content

feat(core): allow synchronous values for stream Resources#67382

Merged
kirjs merged 1 commit intoangular:mainfrom
JeanMeche:resource-stream-sync
Apr 15, 2026
Merged

feat(core): allow synchronous values for stream Resources#67382
kirjs merged 1 commit intoangular:mainfrom
JeanMeche:resource-stream-sync

Conversation

@JeanMeche
Copy link
Copy Markdown
Member

@JeanMeche JeanMeche commented Mar 2, 2026

In order for resources to allow caching in SSR context (eg in the TransferState), resource need to be able to set their value synchronously.

If the resource value is not set synchronously, the resource will be in in a "loading" state which is responsible for destroying the server-hydrated resolved DOM.

Related to #62897

@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: core Issues related to the framework runtime labels Mar 2, 2026
@ngbot ngbot bot added this to the Backlog milestone Mar 2, 2026
@JeanMeche JeanMeche force-pushed the resource-stream-sync branch 18 times, most recently from 36e3d85 to 2ce9752 Compare March 2, 2026 21:29
Comment thread packages/core/rxjs-interop/test/rx_resource_spec.ts
Comment thread packages/core/rxjs-interop/test/rx_resource_spec.ts
@JeanMeche JeanMeche requested a review from alxhub March 2, 2026 21:32
@JeanMeche JeanMeche marked this pull request as ready for review March 2, 2026 21:32
@pullapprove pullapprove bot requested a review from atscott March 2, 2026 21:32
@atscott
Copy link
Copy Markdown
Contributor

atscott commented Mar 2, 2026

else users will observe flickers when resource switches between 'loading'/'resolved'

I think it should be clear that this goes deeper than a "flicker". its first render will destroy the server-hydrated resolved DOM. This might cause flashing (or it might not) but it does cause event replay, user interaction in the meantime, etc to be lost. The flicker could be addressed in a different way (e.g. not having macrotask scheduling) but the hydration issue cannot be addressed by a change to scheduling.

@JeanMeche JeanMeche force-pushed the resource-stream-sync branch from 2ce9752 to 2071eab Compare March 2, 2026 23:06
@JeanMeche JeanMeche mentioned this pull request Mar 3, 2026
@JeanMeche JeanMeche force-pushed the resource-stream-sync branch from 2071eab to d702b7b Compare April 2, 2026 23:52
@JeanMeche JeanMeche force-pushed the resource-stream-sync branch from d702b7b to a4b2585 Compare April 3, 2026 00:30

// @public
export type ResourceStreamingLoader<T, R> = (param: ResourceLoaderParams<R>) => PromiseLike<Signal<ResourceStreamItem<T>>>;
export type ResourceStreamingLoader<T, R> = (param: ResourceLoaderParams<R>) => Signal<ResourceStreamItem<T>> | PromiseLike<Signal<ResourceStreamItem<T>>> | undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Should we have a helper type in core somewhere that's type Awaitable<T> = T | PromiseLike<T>;? I think this could be useful elsewhere as well. It's too bad there isn't a built in like there is "Awaited"

@pullapprove pullapprove bot requested a review from atscott April 3, 2026 16:15
return;
}

this.state.set({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can share this with the block below if we rearrange things so stream is the final signal and we use something like rawStream or loaderResult for the raw loader output. It'd make the flow here clearer.

@pullapprove pullapprove bot requested a review from thePunderWoman April 7, 2026 21:27
Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

@JeanMeche JeanMeche force-pushed the resource-stream-sync branch from a4b2585 to 4ca1ece Compare April 8, 2026 07:21
@JeanMeche JeanMeche added the action: global presubmit The PR is in need of a google3 global presubmit label Apr 8, 2026
In order for resources to allow caching in SSR context (eg in the TransferState), resource need to be able to set their value synchronously.

If the resource value is not set synchronously, the resource will be in in a "loading" state which is responsible for destroying the server-hydrated resolved DOM.
@JeanMeche JeanMeche force-pushed the resource-stream-sync branch from 87a6c47 to ddcf536 Compare April 9, 2026 11:47
@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Apr 14, 2026
@JeanMeche
Copy link
Copy Markdown
Member Author

JeanMeche commented Apr 14, 2026

caretaker note: I'll need to submit cl/877332112 at the same time as this change

@JeanMeche JeanMeche added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Apr 14, 2026
@kirjs kirjs merged commit 4e33106 into angular:main Apr 15, 2026
25 of 27 checks passed
@kirjs
Copy link
Copy Markdown
Contributor

kirjs commented Apr 15, 2026

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: global presubmit The PR is in need of a google3 global presubmit action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime detected: feature PR contains a feature commit merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants