GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — docs: refactor pipe example to use the HttpClient by danielsogl · Pull Request #22741 · angular/angular

Conversation

@danielsogl

@danielsogl danielsogl commented Mar 13, 2018

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

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

What is the current behavior?

Actually the pipes example is using the deprecated HttpModule.

What is the new behavior?

Refactored the example to use the new HttpClient

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@googlebot googlebot added the cla: yes label Mar 13, 2018
@kara kara added the comp: docs label Mar 13, 2018
@trotyl

trotyl commented Mar 14, 2018

Copy link
Copy Markdown
Contributor

Partially covered by #22727

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ngbot

ngbot Bot commented Mar 20, 2018

Copy link
Copy Markdown

Hi @danielsogl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

4 similar comments
@ngbot

ngbot Bot commented Mar 20, 2018

Copy link
Copy Markdown

Hi @danielsogl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ngbot

ngbot Bot commented Mar 20, 2018

Copy link
Copy Markdown

Hi @danielsogl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ngbot

ngbot Bot commented May 10, 2018

Copy link
Copy Markdown

Hi @danielsogl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ngbot

ngbot Bot commented May 10, 2018

Copy link
Copy Markdown

Hi @danielsogl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brandonroberts

brandonroberts commented Jul 5, 2018

Copy link
Copy Markdown
Contributor

@danielsogl will you rebase on master and remove (aio) from the git commit message?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brandonroberts brandonroberts self-assigned this Jul 5, 2018
@brandonroberts brandonroberts added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews target: patch This PR is targeted for the next patch release labels Jul 5, 2018
@danielsogl
danielsogl force-pushed the refactor-example-pipe branch from fa3a966 to a0fde4e Compare July 5, 2018 15:33
@danielsogl danielsogl changed the title docs(aio): refactor pipe example to use the HttpClient docs(): refactor pipe example to use the HttpClient Jul 5, 2018
@danielsogl
danielsogl force-pushed the refactor-example-pipe branch from a0fde4e to bc6b0cf Compare July 5, 2018 15:34
@danielsogl

danielsogl commented Jul 5, 2018

Copy link
Copy Markdown
Contributor Author

@brandonroberts Done!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brandonroberts

brandonroberts commented Jul 5, 2018

Copy link
Copy Markdown
Contributor

Update the commit message to start with docs: ...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@danielsogl
danielsogl force-pushed the refactor-example-pipe branch from bc6b0cf to 92eec47 Compare July 5, 2018 18:54
@danielsogl danielsogl changed the title docs(): refactor pipe example to use the HttpClient docs: refactor pipe example to use the HttpClient Jul 5, 2018
@danielsogl

danielsogl commented Jul 5, 2018

Copy link
Copy Markdown
Contributor Author

@brandonroberts fixed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

brandonroberts
brandonroberts previously approved these changes Jul 11, 2018
View reviewed changes
@brandonroberts brandonroberts added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jul 11, 2018
@ngbot

ngbot Bot commented Jul 11, 2018

Copy link
Copy Markdown

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "continuous-integration/travis-ci/pr" is failing
    pending status "code-review/pullapprove" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brandonroberts
brandonroberts dismissed their stale review July 11, 2018 13:34

Cleanup

@brandonroberts

brandonroberts commented Jul 11, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

@danielsogl will you rebase this branch on master? Some of these changes have already landed in master

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brandonroberts brandonroberts 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: merge The PR is ready for merge by the caretaker labels Jul 11, 2018
@danielsogl
danielsogl force-pushed the refactor-example-pipe branch from 379f7e6 to 2d7e1ba Compare July 11, 2018 16:31
@brandonroberts

brandonroberts commented Jul 18, 2018

Copy link
Copy Markdown
Contributor

@danielsogl something went off track with your rebase. Will you rebase on master again?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@danielsogl
danielsogl force-pushed the refactor-example-pipe branch from 2d7e1ba to 35032da Compare July 18, 2018 20:29
brandonroberts
brandonroberts suggested changes Jul 20, 2018
View reviewed changes

@brandonroberts brandonroberts left a comment

Copy link
Copy Markdown
Contributor

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.

There are still some changes outside the aio folder that need to be reverted.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread aio/content/examples/pipes/src/app/app.module.ts Outdated

@brandonroberts brandonroberts Jul 20, 2018

Copy link
Copy Markdown
Contributor

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.

An HttpClientModule import already exists

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@danielsogl danielsogl Jul 20, 2018

Copy link
Copy Markdown
Contributor Author

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.

Fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread aio/content/examples/pipes/src/app/app.module.ts Outdated

@brandonroberts brandonroberts Jul 20, 2018

Copy link
Copy Markdown
Contributor

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.

This should not be removed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@danielsogl danielsogl Jul 20, 2018

Copy link
Copy Markdown
Contributor Author

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.

Fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread aio/content/examples/pipes/src/app/fetch-json.pipe.ts Outdated

@brandonroberts brandonroberts Jul 20, 2018

Copy link
Copy Markdown
Contributor

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.

The from statements should be lined up. This is intentional.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@danielsogl
danielsogl force-pushed the refactor-example-pipe branch 2 times, most recently from f69c335 to d2eca35 Compare July 20, 2018 12:23
@jenniferfell jenniferfell added the state: community Someone from the Angular community is working on this issue or submitted this PR label Jul 23, 2018
@danielsogl
danielsogl force-pushed the refactor-example-pipe branch from d2eca35 to 49bbd6c Compare July 25, 2018 05:36
@danielsogl

danielsogl commented Jul 25, 2018

Copy link
Copy Markdown
Contributor Author

@brandonroberts rebased on master again

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gkalpak
gkalpak suggested changes Jul 25, 2018
View reviewed changes
Comment thread aio/content/examples/pipes/src/app/app.module.ts Outdated

@gkalpak gkalpak Jul 25, 2018

Copy link
Copy Markdown
Member

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.

Please don't remove the spaces (here and above) for consistency.
(I don't like them either, but it is preferrable to remain consistent across examples and most examples use them.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread aio/content/examples/pipes/src/app/fetch-json.pipe.ts Outdated

@gkalpak gkalpak Jul 25, 2018

Copy link
Copy Markdown
Member

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.

Keep the space.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread aio/content/examples/pipes/src/app/fetch-json.pipe.ts Outdated

@gkalpak gkalpak Jul 25, 2018

Copy link
Copy Markdown
Member

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.

Why the parens?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread aio/content/guide/architecture-components.md Outdated

@gkalpak gkalpak Jul 25, 2018

Copy link
Copy Markdown
Member

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.

Revert.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread modules/benchmarks/src/tree/render3/tree.ts Outdated

@gkalpak gkalpak Jul 25, 2018

Copy link
Copy Markdown
Member

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.

These changes seem irrelevant 😁
(Probably a rebase gone bad.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@danielsogl
danielsogl force-pushed the refactor-example-pipe branch from 94aedd3 to 7312f8c Compare July 25, 2018 10:10
@danielsogl
docs: refactor pipe example to use the HttpClient
db21c38
docs: fix typos in 'Httpclient' docs (angular#19127)

PR Close angular#19127

docs(router): add `paramsInheritanceStrategy` documentation (angular#22590)

PR Close angular#22590

docs: add app.module to changed documents (angular#23876)

PR Close angular#23876

docs: clarify faqs about services (angular#24079)

PR Close angular#24079

docs(aio): added a link to Angular Zero online course (Traditional Chinese) (angular#24228)

PR Close angular#24228

docs: describe rounding behaviour of 'DecimalPipe' (angular#24303)

PR Close angular#24303

docs(common): fix in the documentation of PUT (angular#24528)
PR Close angular#24528

docs: add workspace and related cli terms (angular#24633)

PR Close angular#24633

docs(changelog): correct inaccuracies (angular#24713)
PR Close angular#24713

test: integration test for TS 2.9.x (angular#24749)

PR Close angular#24749

fix(ivy): pipes are pure by default (angular#24750)

PR Close angular#24750

build(bazel): update to rule_nodejs 0.10.0 (angular#24759)

PR Close angular#24759

build: upgrade jasmine (and related typings) to latest version (angular#19904)

With these changes, the types are a little stricter now and also not
compatible with Protractor's jasmine-like syntax. So, we have to also
use `@types/jasminewd2` for e2e tests (but not for non-e2e tests).

I also had to "augment" `@types/jasminewd2`, because the latest
typings from [DefinitelyTyped][1] do not reflect the fact that the
`jasminewd2` version (v2.1.0) currently used by Protractor supports
passing a `done` callback to a spec.

[1]: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/566e0394859fdc1dc893658ccec6b06372d56a91/types/jasminewd2/index.d.ts#L9-L15

Fixes angular#23952
Closes angular#24733

PR Close angular#19904

test: make `NgMatchers` type-aware (angular#19904)

PR Close angular#19904

refactor: infer type for `it()` assertion functions (angular#19904)

PR Close angular#19904

build: upgrade jasmine to 2.99.x and fix tests (angular#19904)

PR Close angular#19904

refactor: re-organize and "modernize" cjs-jasmine scripts (angular#19904)

PR Close angular#19904

test: run unit tests in random order (angular#19904)

PR Close angular#19904

build: upgrade jasmine to 3.1.0 (angular#19904)

PR Close angular#19904

build: upgrade karma and related dependencies (angular#19904)

PR Close angular#19904

docs: refactored ng-container code (angular#22742)


PR Close angular#22742

docs: update Angular Boot Camp description (angular#23653)

PR Close angular#23653

feat(service-worker): add support for `?` in SW config globbing (angular#24105)

The globbing is used in the following sections:
- `assetGroups` > `resources` > `files`/`versionedFiles`
- `assetGroups` > `resources` > `urls`
- `dataGroups` > `urls`
- `navigationUrls`

Query params are ignored for `files`/`versionedFiles` and
`navigationUrls`, but they are still taken into account for
`assetGroups`/`dataGroups` `urls`. To avoid a breaking change, `?` is
matched literally for these patterns.

PR Close angular#24105

refactor(service-worker): avoid unnecessary operations and remove unused code (angular#24127)

PR Close angular#24127

fix(service-worker): avoid network requests when looking up hashed resources in cache (angular#24127)

PR Close angular#24127

feat(ivy): properly apply style="", [style], [style.foo] and [attr.style] bindings (angular#24602)

PR Close angular#24602

fix(language-service): do not overwrite native `Reflect` (angular#24299)

Fixes angular#21420

PR Close angular#24299

fix(common): use correct currency format for locale de-AT (angular#24658)

Fixes angular#24609
PR Close angular#24658

fix(ivy): correctly resolve Array  property access (angular#24664)

PR Close angular#24664

build: make `internal-angular` karma reporter compatible with latest karma (angular#24803)

Due to changes in [email protected], `internal-angular` karma reporter stopped
showing browser logs (such as `console.log()` etc.).
Related to d571a51.

PR Close angular#24803

build: remove unnecessary `internal-angular` karma reporter (angular#24803)

The reporter was added in 87d56ac, with the purpose of fixing
source-map paths (which was apparently needed back then). Things have
moved around a lot since then and the custom reporter doesn't seem to be
necessary any more. By removing the reporter, we have one less thing to
worry about while upgrading karma; plus we get improvements in built-in
reporters for free.

Output with the custom reporter:
```
at someMethod (packages/core/.../some-file.ts:13:37)
```

Output with the built-in reporter:
```
at someMethod (packages/core/.../some-file.ts:13.37 <- dist/all/@angular/core/.../some-file.js:1:337)
```

PR Close angular#24803

fix(core): mark NgModule as not the root if APP_ROOT is set to false (angular#24814)

Tree shakable providers use the APP_ROOT token to determine where to attach themselves. APP_ROOT gets set on NgModule with BrowserModule irrespective of whether it is actually the root(Ex. in case of SSR app where the shell app is first bootstrapped without BrowserModule being the root module).

This change allows a NgModule with BrowserModule to explicitly mark itself as not the root by setting APP_ROOT token to false. This allows tree shakable providers to be attached to the right rott module.

PR Close angular#24814

fix(platform-browser): mark Meta and Title services as tree shakable providers (angular#24815)

This lets services that use Meta and Title services to be tree shakable and provided in root.

PR Close angular#24815

docs(aio): unified console.log single string style (angular#22737)
PR Close angular#22737

docs: unified console.log single string style (angular#22737)


PR Close angular#22737

fix(compiler-cli): Use typescript to resolve modules for metadata (angular#22856)

The current module resolution simply attaches .ts to the import/export path, which does
not work if the path is using Node / CommonJS behavior to resolve to an index.ts file.
This patch uses typescript's module resolution logic, and will attempt to load the original
typescript file if this resolution returns a .js or .d.ts file

PR Close angular#22856

docs: clarify wording in architecture overview (angular#24481)

Closes angular#23463
Closes angular#22158

PR Close angular#24481

docs: add tree-shakable providers (angular#24481)

PR Close angular#24481

fix(ivy): support projecting into dynamic views (angular#24752)

PR Close angular#24752

refactor(ivy): replace pNextOrParent with TNode props (angular#24752)

PR Close angular#24752

docs: fix typo in Universal guide (angular#24812)

PR Close angular#24812

fix(platform-browser): workaround wrong import path generated by ngc for DOCUMENT (angular#24830)


docs: correct project definition (angular#24807)

PR Close angular#24807

build: update to latest nodejs bazel rules (angular#24817)

PR Close angular#24817

fix(common): do not round factional seconds (angular#24831)

fixes angular#24384

PR Close angular#24831

docs: fix typos referencing inline component styles (angular#22557)

PR Close angular#22557

fix(router): add ability to recover from malformed url (angular#23283)

Fixes angular#21468

PR Close angular#23283

docs: fix incorrect forms selector references (angular#22631)

PR Close angular#22631

docs(aio): unified string chaining (angular#22735)
PR Close angular#22735

docs: unified string chaining (angular#22735)


PR Close angular#22735

docs(forms): added missing backtick (angular#24451)

Fixed trivial markdown problem with a missing backtick.

PR Close angular#24451

docs(forms): update API reference for form validators (angular#24734)

PR Close angular#24734

Merge remote-tracking branch 'upstream/master' into refactor-example-pipe


docs: refactor pipe example to use the HttpClient


docs: refactor pipe example to use the HttpClient


docs: refactor pipe example to use the HttpClient
@danielsogl
danielsogl force-pushed the refactor-example-pipe branch from 7312f8c to db21c38 Compare July 25, 2018 10:12
@danielsogl

danielsogl commented Jul 25, 2018

Copy link
Copy Markdown
Contributor Author

@gkalpak yes I failed with the rebase. Now it should be fine

gkalpak reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gkalpak
gkalpak approved these changes Jul 25, 2018
View reviewed changes

@gkalpak gkalpak left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

Looks great 😃 Thx! 👍
(Leaving it to @brandonroberts to give the final LGTM - assuming CI is green 🤔)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
brandonroberts
brandonroberts approved these changes Jul 27, 2018
View reviewed changes
@brandonroberts brandonroberts added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jul 27, 2018
@IgorMinar IgorMinar closed this in 5399c17 Jul 30, 2018
IgorMinar pushed a commit that referenced this pull request Jul 30, 2018
@danielsogl @IgorMinar
docs: refactor pipe example to use the HttpClient (#22741)
9a6d26e
PR Close #22741
IgorMinar pushed a commit that referenced this pull request Jul 30, 2018
@danielsogl @IgorMinar
docs: refactor pipe example to use the HttpClient (#22741)
f5e1802
PR Close #22741
@danielsogl
danielsogl deleted the refactor-example-pipe branch July 31, 2018 05:08
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 13, 2019

Copy link
Copy Markdown

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@brandonroberts brandonroberts brandonroberts approved these changes
@gkalpak gkalpak gkalpak approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@brandonroberts brandonroberts

Labels

action: merge The PR is ready for merge by the caretaker cla: yes state: community Someone from the Angular community is working on this issue or submitted this PR target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@danielsogl @trotyl @brandonroberts @gkalpak @kara @googlebot @jenniferfell
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.