Deprecate withServerTransition call and generate static APP_ID
#49422
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bf94712 to
30edba2
Compare
Member
|
Nit: You could add an entry to the depreciation guide. |
187ca88 to
8a6c8fe
Compare
AndrewKushnir
approved these changes
Mar 14, 2023
Contributor
There was a problem hiding this comment.
nit: we can extract this value to a separate const to make it more self-documenting:
Suggested change
| factory: () => 'ng', | |
| factory: () => DEFAULT_APP_ID, |
Contributor
There was a problem hiding this comment.
If it's just static, can we use a value provider? useValue: DEFAULT_APP_ID
Contributor
Author
There was a problem hiding this comment.
In that case, the default value, will be set in a totally different package since an InjectorToken doesn’t allow useValue.
Ex:
Contributor
There was a problem hiding this comment.
Should we also remove this call (in the same file)?
angular/packages/platform-server/src/utils.ts
Line 242 in ed110a0
Contributor
Author
There was a problem hiding this comment.
I did not remove this one as this overload of renderApplication will be deleted in a follow up PR.
jessicajaniuk
approved these changes
Mar 14, 2023
Contributor
jessicajaniuk
left a comment
There was a problem hiding this comment.
reviewed-for: public-api
atscott
reviewed
Mar 14, 2023
Contributor
Author
I’ll add a breaking change message. |
This commit deprecated ` BrowserModule.withServerTransition` instead `APP_ID` should be used instead to configure the app id.
DEPRECATED: `BrowserModule.withServerTransition` has been deprecated. `APP_ID` should be used instead to set the application ID.
NB: Unless, you render multiple Angular applications on the same page, setting an application ID is not necessary.
Before:
```ts
imports: [
BrowserModule.withServerTransition({ appId: 'serverApp' }),
...
]
```
After:
```ts
imports: [
BrowserModule,
{ provide: APP_ID, useValue: 'serverApp' },
...
],
```
Prior to this change, a random application ID was generated each time which forced users using server rendering to provide an application ID themselves. This was needed to handle rare cases when multiple Angular applications are rendered on the same page.
With this change the application ID is no longer generated randomly and instead it is hard coded.
BREAKING CHANGE:
The `APP_ID` token value is no longer randomly generated. If you are bootstrapping multiple application on the same page you will need to set to provide the `APP_ID` yourself.
```ts
bootstrapApplication(ComponentA, {
providers: [
{ provide: APP_ID, useValue: 'app-a' },
// ... other providers ...
]
});
```
This commit removes the usages of `withServerTransition` form tests.
df29310 to
915ab07
Compare
Contributor
Author
|
@atscott, PTAL. |
AndrewKushnir
approved these changes
Mar 15, 2023
Contributor
AndrewKushnir
left a comment
There was a problem hiding this comment.
Reviewed-for: public-api
Member
|
This PR was merged into the repository by commit cf98777. |
alxhub
pushed a commit
that referenced
this pull request
Mar 16, 2023
Prior to this change, a random application ID was generated each time which forced users using server rendering to provide an application ID themselves. This was needed to handle rare cases when multiple Angular applications are rendered on the same page.
With this change the application ID is no longer generated randomly and instead it is hard coded.
BREAKING CHANGE:
The `APP_ID` token value is no longer randomly generated. If you are bootstrapping multiple application on the same page you will need to set to provide the `APP_ID` yourself.
```ts
bootstrapApplication(ComponentA, {
providers: [
{ provide: APP_ID, useValue: 'app-a' },
// ... other providers ...
]
});
```
PR Close #49422
alxhub
pushed a commit
that referenced
this pull request
Mar 16, 2023
This commit removes the usages of `withServerTransition` form tests. PR Close #49422
alan-agius4
added a commit
to alan-agius4/angular-cli
that referenced
this pull request
Mar 24, 2023
…rverTransition` This commit removes generation of `.withServerTransition` in the universal schematic as is deprecated. DEPRECATED: the `app-id` option in the app-shell and universal schematics has been deprecated without replacement. See: angular/angular#49422 for more information about the rational of this change.
alan-agius4
added a commit
to alan-agius4/angular-cli
that referenced
this pull request
Mar 24, 2023
…rverTransition` This commit removes generation of `.withServerTransition` in the universal schematic as is deprecated. DEPRECATED: the `app-id` option in the app-shell and universal schematics has been deprecated without replacement. See: angular/angular#49422 for more information about the rational of this change.
angular-robot bot
pushed a commit
to angular/angular-cli
that referenced
this pull request
Mar 24, 2023
…rverTransition` This commit removes generation of `.withServerTransition` in the universal schematic as is deprecated. DEPRECATED: the `app-id` option in the app-shell and universal schematics has been deprecated without replacement. See: angular/angular#49422 for more information about the rational of this change.
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat(platform-browser): deprecate
withServerTransitioncallThis commit deprecated
BrowserModule.withServerTransitioninsteadAPP_IDshould be used instead to configure the app id.DEPRECATED:
BrowserModule.withServerTransitionhas been deprecated.APP_IDshould be used instead to set the application ID.NB: Unless, you render multiple Angular applications on the same page, setting an application ID is not necessary.
Before:
After:
refactor(core): generate a static application ID.
Prior to this change, a random application ID was generated each time which forced users using server rendering to provide an application ID themselves. This was needed to handle rare cases when multiple Angular applications are rendered on the same page.
With this change the application ID is no longer generated randomly and instead it is hard coded.
BREAKING CHANGE:
The
APP_IDtoken value is no longer randomly generated. If you are bootstrapping multiple application on the same page you will need to set to provide theAPP_IDyourself.