Deprecate `withServerTransition` call and generate static `APP_ID` by alan-agius4 · Pull Request #49422 · angular/angular

alan-agius4 · 2023-03-14T10:18:11Z

feat(platform-browser): deprecate withServerTransition call
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:

imports: [
  BrowserModule.withServerTransition({ appId: 'serverApp' }),
  ...
]

After:

imports: [
  BrowserModule,
  { provide: APP_ID, useValue: 'serverApp' },
  ...
],

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_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.

bootstrapApplication(ComponentA, {
  providers: [
   { provide: APP_ID, useValue: 'app-a' },
   // ... other providers ...
  ]
});

JeanMeche · 2023-03-14T15:10:59Z

Nit: You could add an entry to the depreciation guide.

AndrewKushnir

Looks great! 👍

packages/core/src/application_tokens.ts

AndrewKushnir · 2023-03-14T18:58:32Z

packages/core/src/application_tokens.ts

nit: we can extract this value to a separate const to make it more self-documenting:

Suggested change

factory: () => 'ng',

factory: () => DEFAULT_APP_ID,

If it's just static, can we use a value provider? useValue: DEFAULT_APP_ID

In that case, the default value, will be set in a totally different package since an InjectorToken doesn’t allow useValue.

Ex:

angular/packages/platform-browser/src/browser.ts

Line 200 in ed110a0

{provide: INJECTOR_SCOPE, useValue: 'root'},

AndrewKushnir · 2023-03-14T18:59:48Z

packages/platform-server/src/utils.ts

Should we also remove this call (in the same file)?

angular/packages/platform-server/src/utils.ts

Line 242 in ed110a0

importProvidersFrom(BrowserModule.withServerTransition({appId})),

I did not remove this one as this overload of renderApplication will be deleted in a follow up PR.

jessicajaniuk

reviewed-for: public-api

atscott

Is the second commit (refactor(core): generate a static application ID.) a breaking change for when there are multiple applications (however rare that would be)

alan-agius4 · 2023-03-14T19:19:56Z

Is the second commit (refactor(core): generate a static application ID.) a breaking change for when there are multiple applications (however rare that would be)

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.

alan-agius4 · 2023-03-15T06:47:43Z

@atscott, PTAL.

AndrewKushnir

Reviewed-for: public-api

alxhub · 2023-03-16T00:08:20Z

This PR was merged into the repository by commit cf98777.

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

This commit removes the usages of `withServerTransition` form tests. PR Close #49422

…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-automatic-lock-bot · 2023-04-16T00:08:46Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

alan-agius4 added the target: major This PR is targeted for the next major release label Mar 14, 2023

angular-robot bot added detected: deprecation PR contains a commit with a deprecation detected: feature PR contains a feature commit labels Mar 14, 2023

alan-agius4 force-pushed the app_id_deprecate branch 2 times, most recently from bf94712 to 30edba2 Compare March 14, 2023 11:56

alan-agius4 marked this pull request as ready for review March 14, 2023 12:39

alan-agius4 requested a review from AndrewKushnir March 14, 2023 12:40

alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer area: server Issues related to server-side rendering labels Mar 14, 2023

ngbot bot added this to the Backlog milestone Mar 14, 2023

alan-agius4 added the area: core Issues related to the framework runtime label Mar 14, 2023

alan-agius4 force-pushed the app_id_deprecate branch from 187ca88 to 8a6c8fe Compare March 14, 2023 16:28

AndrewKushnir approved these changes Mar 14, 2023

View reviewed changes

pullapprove bot requested review from dylhunn, jessicajaniuk and pkozlowski-opensource March 14, 2023 19:00

jessicajaniuk approved these changes Mar 14, 2023

View reviewed changes

pullapprove bot requested a review from atscott March 14, 2023 19:09

atscott reviewed Mar 14, 2023

View reviewed changes

pullapprove bot requested a review from atscott March 14, 2023 19:16

alan-agius4 added 3 commits March 15, 2023 06:44

test: update tests to remove deprecated withServerTransition

915ab07

This commit removes the usages of `withServerTransition` form tests.

alan-agius4 force-pushed the app_id_deprecate branch from df29310 to 915ab07 Compare March 15, 2023 06:46

angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Mar 15, 2023

AndrewKushnir approved these changes Mar 15, 2023

View reviewed changes

alan-agius4 removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Mar 15, 2023

alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Mar 15, 2023

alxhub closed this in 630af63 Mar 16, 2023

alxhub pushed a commit that referenced this pull request Mar 16, 2023

test: update tests to remove deprecated withServerTransition (#49422)

cf98777

This commit removes the usages of `withServerTransition` form tests. PR Close #49422

alan-agius4 deleted the app_id_deprecate branch March 16, 2023 05:59

alan-agius4 mentioned this pull request Mar 24, 2023

feat(@schematics/angular): remove generation of BrowserModule.withServerTransition angular/angular-cli#24909

Merged

angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 16, 2023

Conversation

alan-agius4 commented Mar 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeanMeche commented Mar 14, 2023

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AndrewKushnir Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

atscott Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

alan-agius4 Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

alan-agius4 Mar 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jessicajaniuk left a comment

Choose a reason for hiding this comment

Uh oh!

atscott left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alan-agius4 commented Mar 14, 2023

Uh oh!

alan-agius4 commented Mar 15, 2023

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

alxhub commented Mar 16, 2023

Uh oh!

angular-automatic-lock-bot bot commented Apr 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alan-agius4 commented Mar 14, 2023 •

edited

Loading

alan-agius4 Mar 14, 2023 •

edited

Loading

atscott left a comment •

edited

Loading