-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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(server): renderApplication
now accepts a bootstrapping method
#49248
feat(server): renderApplication
now accepts a bootstrapping method
#49248
Conversation
@@ -187,20 +212,41 @@ export function renderModule<T>(moduleType: Type<T>, options: { | |||
* @developerPreview | |||
*/ | |||
export function renderApplication<T>(rootComponent: Type<T>, options: { | |||
/** @deprecated use `APP_ID` token to set the application ID. */ | |||
appId: string, | |||
document?: string|Document, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this API is developer preview, can we just remove instead of deprecating?
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct and also if we are going to keep the current method to pass in a component type deprecate passing in the APP_ID
.
I am okay with keeping it for now. Although I did have to add an additional private export to core to check if it's a bootstrap function or a component type.
4784b32
to
420977f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alan-agius4 looks great 👍 Just left a couple non-blocking comments.
@@ -187,20 +212,41 @@ export function renderModule<T>(moduleType: Type<T>, options: { | |||
* @developerPreview | |||
*/ | |||
export function renderApplication<T>(rootComponent: Type<T>, options: { | |||
/** @deprecated use `APP_ID` token to set the application ID. */ | |||
appId: string, | |||
document?: string|Document, |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
OOC what is this trying to solve? Sorry if it isn't obvious for me |
035921b
to
04fed46
Compare
@alfaproject This is needed for a cleaner integration when using SSR and SSG. Where users do not have direct access to |
There was a problem hiding this 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
There was a problem hiding this 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
@alan-agius4 it looks like this PR has a merge conflict with the main branch right now (likely due to another PR that removed |
…g method The `renderApplication` now also accepts a bootstrapping function call with return `Promise<ApplicationRef>`as the first parameter. Example: ```ts const bootstrap = () => bootstrapApplication(RootComponent, appConfig); const output: string = await renderApplication(bootstrap); ```
04fed46
to
be34a3c
Compare
@AndrewKushnir rebased. Caretaker note: I don’t think this required another presubmit seeing that it’s just a rebase and the changes. |
This PR was merged into the repository by commit b5278cc. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
The
renderApplication
now also accepts a bootstrapping function call with returnPromise<ApplicationRef>
as the first parameter.Example: