docs: update `universal` guide and docs example to v9 by gkalpak · Pull Request #36483 · angular/angular

gkalpak · 2020-04-07T15:32:30Z

This PR updates the universal guide and docs example to be aligned with what the v9 CLI generates. It also removes an obsolete section (and the corresponding code) and adds some tests for the example app.

See the individual commit messages for more details.

Fixes #35289.

This was originally part of #34374 (and split off for easier reviewing).
Partially addresses: FW-1609

mary-poppins · 2020-04-07T15:41:56Z

You can preview 0fccbbf at https://pr36483-0fccbbf.ngbuilds.io/.

mary-poppins · 2020-04-07T16:03:16Z

You can preview 8516692 at https://pr36483-8516692.ngbuilds.io/.

IgorMinar

overall LGTM! thanks!

IgorMinar · 2020-04-08T07:01:42Z

Suggested change

### Using absolute URLs for server requests

### Using absolute URLs for HTTP (data) requests on the server

IgorMinar · 2020-04-08T07:03:55Z

Suggested change

intercept(req: HttpRequest<any>, next: HttpHandler) {

intercept(req: HttpRequest<unknown>, next: HttpHandler) {

Can you please use unknown whenever we can instead of any as any can lead to lots of type inference issues... thanks!

I just move the code snippet from the guide to a file, so I kept the original code. I assume any was used to match the HttpInterceptor#intercept() signature (which features any).

In any case, this code is removed in the following fixup commit, so this doesn't really matter 😁
I'll merge the fixup commit into this one to avoid confusion.

IgorMinar

Reviewed-for: global-approvers

but let's have @aikidave also take a quick look to check the language. thanks!

mary-poppins · 2020-04-08T12:55:12Z

You can preview 11cbc21 at https://pr36483-11cbc21.ngbuilds.io/.

aikithoughts

A few suggestions to make the text a little shorter and easier to read.

aikithoughts · 2020-04-14T23:06:05Z

Suggested change

So, you don't need to do anything to make relative URLs work on the server.

You don't need to do anything to make relative URLs work on the server.

aikithoughts · 2020-04-14T23:07:24Z

Suggested change

The recommended solution, which is also the least intrusive as it does not require any changes to the app, is to pass the full request URL to the `options` argument of [renderModule()](api/platform-server/renderModule) or [renderModuleFactory()](api/platform-server/renderModuleFactory) (depending on what you use to render `AppServerModule` on the server).

The recommended solution is to pass the full request URL to the `options` argument of [renderModule()](api/platform-server/renderModule) or [renderModuleFactory()](api/platform-server/renderModuleFactory) (depending on what you use to render `AppServerModule` on the server). This option is the least intrusive as it does not require any changes to the app,

aikithoughts · 2020-04-14T23:08:03Z

Suggested change

Now, on every HTTP request made as part of rendering the app on the server, Angular will be able to correctly resolve the request URL to an absolute URL, using the provided `options.url`.

Now, on every HTTP request made as part of rendering the app on the server, Angular can correctly resolve the request URL to an absolute URL, using the provided `options.url`.

aikithoughts · 2020-04-14T23:08:52Z

Unless I am missing something, it appears that we say that there are several ways to handle relative URLs, but only provide one, as it is the recommended way to do it. If that's the case, we can delete line 282.

…s example ZIP archive Previously, the `package.json` template used in the ZIP archive of the `universal` example that we offer for download missed the `build` npm script. This commit updates the template for the `universal` docs example to include the `build` npm script. NOTE: The `build` npm script is already included in `aio/tools/examples/shared/boilerplate/universal/package.json`, but it was removed by the example zipper.

gkalpak · 2020-04-15T10:58:31Z

Thx, @aikidave! I've made the changes you suggested.

mary-poppins · 2020-04-15T11:06:44Z

You can preview 928dd02 at https://pr36483-928dd02.ngbuilds.io/.

alan-agius4

LGTM, from universal side. Thanks @gkalpak.

I noticed these minor styling issues while aligning the `universal` examples with the `toh-pt6` example (in a subsequent commit). PR Close #36483

…match latest CLI (#36483) Update the `main.ts` files in Tour-of-Heroes examples to match what would be generated by the latest CLI. PR Close #36483

As mentioned in the `universal` guide, the `toh-pt6` examples is the starting poitn for the `universal` example. However, the two examples had become out-of-sync, because some fixes/changes were made to the Tour-of-Heroes examples. This commit ports these changes to the `universal` example. PR Close #36483

…36483) Update the `universal` example (and related files) to match what would be generated by the latest CLI. Fixes #35289 PR Close #36483

… error in `universal` example (#36483) Previously, building the `universal` example failed with: ``` node_modules/@types/express/index.d.ts(90,50): error TS2694: Namespace '".../@types/express-serve-static-core/index"' has no exported member 'Params'. node_modules/@types/express/index.d.ts(90,64): error TS2694: Namespace '".../@types/express-serve-static-core/index"' has no exported member 'ParamsDictionary'. ``` This commit fixes the error by upgrading `@types/express-serve-static-core` to a newer version. See DefinitelyTyped/DefinitelyTyped#40905 for more details. PR Close #36483

Previously, there were no tests for the `universal` docs example. This meant that the project was not tested at all (not even ensuring that it can be built successfully). This commit adds e2e tests for the `universal` example (ported from `toh-pt6` and cleaned up) and also verifies that the project can be built successfully (including the server). PR Close #36483

PR Close #36483

In the past, server-side rendered apps needed to convert URLs used in API requests to absolute when rendering on the server. Originally, this was handled in the `universal` guide and corresponding example app by modifying the `HeroService` to use `APP_BASE_HREF` to derive the absolute URL. In #28956, the guide was updated to show an improved method: Specifying an `HttpInterceptor` that took care of converting the URLs to absolute. That interceptor was only provided when rendering the app on the server. By mistake, the corresponding example app was not updated along with the guide. Since `@nguniversal/*` v7.1.0, it is no longer necessary to convert the URLs to absolute inside the app. This is handled in the `@nguniversal` libs (see angular/universal#897). This commit updates the example app to remove unnecessary code and modifies the guide to mention the issue with absolute URLs, but explain that developers only need to worry about it when not using one of the `@nguniversal/*-engine` packages. PR Close #36483

…s example ZIP archive (#36483) Previously, the `package.json` template used in the ZIP archive of the `universal` example that we offer for download missed the `build` npm script. This commit updates the template for the `universal` docs example to include the `build` npm script. NOTE: The `build` npm script is already included in `aio/tools/examples/shared/boilerplate/universal/package.json`, but it was removed by the example zipper. PR Close #36483

I noticed these minor styling issues while aligning the `universal` examples with the `toh-pt6` example (in a subsequent commit). PR Close #36483

…match latest CLI (#36483) Update the `main.ts` files in Tour-of-Heroes examples to match what would be generated by the latest CLI. PR Close #36483

As mentioned in the `universal` guide, the `toh-pt6` examples is the starting poitn for the `universal` example. However, the two examples had become out-of-sync, because some fixes/changes were made to the Tour-of-Heroes examples. This commit ports these changes to the `universal` example. PR Close #36483

…36483) Update the `universal` example (and related files) to match what would be generated by the latest CLI. Fixes #35289 PR Close #36483

… error in `universal` example (#36483) Previously, building the `universal` example failed with: ``` node_modules/@types/express/index.d.ts(90,50): error TS2694: Namespace '".../@types/express-serve-static-core/index"' has no exported member 'Params'. node_modules/@types/express/index.d.ts(90,64): error TS2694: Namespace '".../@types/express-serve-static-core/index"' has no exported member 'ParamsDictionary'. ``` This commit fixes the error by upgrading `@types/express-serve-static-core` to a newer version. See DefinitelyTyped/DefinitelyTyped#40905 for more details. PR Close #36483

Previously, there were no tests for the `universal` docs example. This meant that the project was not tested at all (not even ensuring that it can be built successfully). This commit adds e2e tests for the `universal` example (ported from `toh-pt6` and cleaned up) and also verifies that the project can be built successfully (including the server). PR Close #36483

PR Close #36483

In the past, server-side rendered apps needed to convert URLs used in API requests to absolute when rendering on the server. Originally, this was handled in the `universal` guide and corresponding example app by modifying the `HeroService` to use `APP_BASE_HREF` to derive the absolute URL. In #28956, the guide was updated to show an improved method: Specifying an `HttpInterceptor` that took care of converting the URLs to absolute. That interceptor was only provided when rendering the app on the server. By mistake, the corresponding example app was not updated along with the guide. Since `@nguniversal/*` v7.1.0, it is no longer necessary to convert the URLs to absolute inside the app. This is handled in the `@nguniversal` libs (see angular/universal#897). This commit updates the example app to remove unnecessary code and modifies the guide to mention the issue with absolute URLs, but explain that developers only need to worry about it when not using one of the `@nguniversal/*-engine` packages. PR Close #36483

angular-automatic-lock-bot · 2020-05-18T16:06:50Z

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

pullapprove Bot requested review from IgorMinar, aikithoughts and alxhub April 7, 2020 15:32

googlebot added the cla: yes label Apr 7, 2020

gkalpak added comp: docs action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Apr 7, 2020

ngbot Bot modified the milestone: needsTriage Apr 7, 2020

gkalpak mentioned this pull request Apr 7, 2020

WIP - build(docs-infra): update docs examples to latest v9 and Ivy #34374

Closed

8 tasks

gkalpak force-pushed the fix-aio-universal-example branch from 0fccbbf to 8516692 Compare April 7, 2020 15:55

gkalpak changed the title ~~docs: update universal guide and docs example~~ docs: update universal guide and docs example to v9 Apr 7, 2020

gkalpak mentioned this pull request Apr 7, 2020

docs: update universal doc #36005

Closed

14 tasks

kara added the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 7, 2020

IgorMinar approved these changes Apr 8, 2020

View reviewed changes

IgorMinar added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 8, 2020

IgorMinar approved these changes Apr 8, 2020

View reviewed changes

IgorMinar removed the request for review from alxhub April 8, 2020 07:07

gkalpak force-pushed the fix-aio-universal-example branch from 8516692 to 3a5e1e7 Compare April 8, 2020 12:38

gkalpak removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Apr 8, 2020

gkalpak force-pushed the fix-aio-universal-example branch from 3a5e1e7 to 11cbc21 Compare April 8, 2020 12:42

aikithoughts added the state: needs doc team review label Apr 8, 2020

atscott added the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 9, 2020

aikithoughts reviewed Apr 14, 2020

View reviewed changes

fixup! docs: remove unneeded code from universal example/guide

928dd02

gkalpak force-pushed the fix-aio-universal-example branch from 11cbc21 to 928dd02 Compare April 15, 2020 10:57

gkalpak removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 15, 2020

alan-agius4 approved these changes Apr 15, 2020

View reviewed changes

gkalpak removed the state: needs doc team review label Apr 16, 2020

This was referenced Apr 16, 2020

Angular SSR #36663

Closed

Angular SSR #36664

Closed

atscott closed this in 9f039d4 Apr 16, 2020

atscott pushed a commit that referenced this pull request Apr 16, 2020

style(docs-infra): clean up Tour-of-Heroes examples (#36483)

061107c

I noticed these minor styling issues while aligning the `universal` examples with the `toh-pt6` example (in a subsequent commit). PR Close #36483

atscott pushed a commit that referenced this pull request Apr 16, 2020

refactor(docs-infra): update universal example to match latest CLI (#…

ec5c108

…36483) Update the `universal` example (and related files) to match what would be generated by the latest CLI. Fixes #35289 PR Close #36483

atscott pushed a commit that referenced this pull request Apr 16, 2020

docs: minor fixes/improvements in the universal guide (#36483)

5a09753

PR Close #36483

atscott pushed a commit that referenced this pull request Apr 16, 2020

style(docs-infra): clean up Tour-of-Heroes examples (#36483)

4f99915

I noticed these minor styling issues while aligning the `universal` examples with the `toh-pt6` example (in a subsequent commit). PR Close #36483

atscott pushed a commit that referenced this pull request Apr 16, 2020

refactor(docs-infra): update universal example to match latest CLI (#…

eec9b6b

…36483) Update the `universal` example (and related files) to match what would be generated by the latest CLI. Fixes #35289 PR Close #36483

atscott pushed a commit that referenced this pull request Apr 16, 2020

docs: minor fixes/improvements in the universal guide (#36483)

609d81c

PR Close #36483

gkalpak deleted the fix-aio-universal-example branch April 17, 2020 12:07

angular-automatic-lock-bot Bot locked and limited conversation to collaborators May 18, 2020

	### Using absolute URLs for server requests
	### Using absolute URLs for HTTP (data) requests on the server

	intercept(req: HttpRequest<any>, next: HttpHandler) {
	intercept(req: HttpRequest<unknown>, next: HttpHandler) {

	So, you don't need to do anything to make relative URLs work on the server.
	You don't need to do anything to make relative URLs work on the server.

	The recommended solution, which is also the least intrusive as it does not require any changes to the app, is to pass the full request URL to the `options` argument of [renderModule()](api/platform-server/renderModule) or [renderModuleFactory()](api/platform-server/renderModuleFactory) (depending on what you use to render `AppServerModule` on the server).
	The recommended solution is to pass the full request URL to the `options` argument of [renderModule()](api/platform-server/renderModule) or [renderModuleFactory()](api/platform-server/renderModuleFactory) (depending on what you use to render `AppServerModule` on the server). This option is the least intrusive as it does not require any changes to the app,

	Now, on every HTTP request made as part of rendering the app on the server, Angular will be able to correctly resolve the request URL to an absolute URL, using the provided `options.url`.
	Now, on every HTTP request made as part of rendering the app on the server, Angular can correctly resolve the request URL to an absolute URL, using the provided `options.url`.

Conversation

gkalpak commented Apr 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mary-poppins commented Apr 7, 2020

Uh oh!

mary-poppins commented Apr 7, 2020

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

mary-poppins commented Apr 8, 2020

Uh oh!

aikithoughts left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkalpak commented Apr 15, 2020

Uh oh!

mary-poppins commented Apr 15, 2020

Uh oh!

alan-agius4 left a comment

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot Bot commented May 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

gkalpak commented Apr 7, 2020 •

edited

Loading