New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(platform-server): provide a way to hook into renderModule* #19023

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@vikerman
Contributor

vikerman commented Sep 4, 2017

A multi BEFORE_APP_SERIALIZED provider can provide function that will be called with the current document just before the document is rendered to
string.

This hook can for example be used for the state transfer module to serialize any server state that needs to be transported to the client, just before the current platform state is rendered to string.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

When using the renderModule* functions from @angular/platform-server there is no way to change the document state after the app is stable but before the current document is serialized to string.

Issue Number: N/A

What is the new behavior?

Users can provide a RENDER_MODULE_HOOK multi-provider that can insert/change document state just before it is rendered.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@vikerman vikerman requested a review from alxhub Sep 4, 2017

@googlebot googlebot added the cla: yes label Sep 4, 2017

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Sep 4, 2017

@Toxicable

This comment has been minimized.

Show comment
Hide comment
@Toxicable

Toxicable Sep 4, 2017

Contributor

Maybe add in support for async function returns like the one I did here https://github.com/angular/angular/pull/19010/files#diff-d6389d42fc011e1bb9b5f6ea6e66faf0R48
I cant think of any great use cases but I don't think there's any harm in doing that

Contributor

Toxicable commented Sep 4, 2017

Maybe add in support for async function returns like the one I did here https://github.com/angular/angular/pull/19010/files#diff-d6389d42fc011e1bb9b5f6ea6e66faf0R48
I cant think of any great use cases but I don't think there's any harm in doing that

@vikerman

This comment has been minimized.

Show comment
Hide comment
@vikerman

vikerman Sep 4, 2017

Contributor

@Toxicable - Ah - We should have sync-ed to avoid duplication. I don't we should be adding more async stuff to the system without proper use case. Also what if the render hook makes the Angular application unstable again? All the async stuff can be done and gated behind the isStable and the render hook should just do sync work.

Contributor

vikerman commented Sep 4, 2017

@Toxicable - Ah - We should have sync-ed to avoid duplication. I don't we should be adding more async stuff to the system without proper use case. Also what if the render hook makes the Angular application unstable again? All the async stuff can be done and gated behind the isStable and the render hook should just do sync work.

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Sep 4, 2017

@Toxicable

This comment has been minimized.

Show comment
Hide comment
@Toxicable

Toxicable Sep 4, 2017

Contributor

I'm always on https://gitter.im/angular/angular if you want an update on what im working on.
I see what you're but isn't the point of this to avoid using isStable?
Although as you mentioned without a solid use case I think it would be best to not support it for now.

Contributor

Toxicable commented Sep 4, 2017

I'm always on https://gitter.im/angular/angular if you want an update on what im working on.
I see what you're but isn't the point of this to avoid using isStable?
Although as you mentioned without a solid use case I think it would be best to not support it for now.

@vikerman

This comment has been minimized.

Show comment
Hide comment
@vikerman

vikerman Sep 4, 2017

Contributor

This is so that modules like state transfer or meta tags adding code can run after isStable but before the final document state is serialized. It is not for avoiding isStable and shouldn't be doing async tasks like http fetch, but just for adding script/meta tags to the document just before it's serialized

Contributor

vikerman commented Sep 4, 2017

This is so that modules like state transfer or meta tags adding code can run after isStable but before the final document state is serialized. It is not for avoiding isStable and shouldn't be doing async tasks like http fetch, but just for adding script/meta tags to the document just before it's serialized

@alxhub

alxhub approved these changes Sep 5, 2017

Show outdated Hide outdated packages/platform-server/src/tokens.ts

@vikerman vikerman requested a review from IgorMinar Sep 5, 2017

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Sep 5, 2017

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Sep 5, 2017

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Sep 5, 2017

feat(platform-server): provide a way to hook into renderModule*
A multi RENDER_MODULE_HOOK provider can provide function that will be called with the current document just before the document is rendered to
string.

This hook can for example be used for the state transfer module to serialize any server state that needs to be transported to the client, just before the current platform state is rendered to string.
@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Sep 5, 2017

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Sep 5, 2017

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