chore(dart): updating to beta.16 #1192
Conversation
Updated docs and samples to beta.16. For this first time, among other things, I payed particular attention to the changes that were made on the `ts` side of things (angular#1178). Since I wrote it up in my notes, here is the check list of `ts` updates with notes about how there might be corresponding changes on the Dart side: - public/docs/_examples/cb-dependency-injection/ts/app/main.ts - public/docs/_examples/cb-ts-to-js/ts/app/main.ts - cb not in Dart docs yet. - public/docs/_examples/dependency-injection/ts/app/car/car-injector.ts - public/docs/_examples/dependency-injection/ts/app/injector.component.ts - Updated Dart example code to avoid null argument. - public/docs/_examples/homepage-hello-world/ts/index.1.html - public/docs/_examples/homepage-tabs/ts/index.1.html - public/docs/_examples/homepage-todo/ts/index.1.html - public/docs/_examples/package.json - N/A for Dart. - public/docs/_examples/pipes/ts/app/app.component.html - public/docs/_examples/pipes/ts/app/app.component.ts - public/docs/_examples/pipes/ts/app/exponential-strength.pipe.ts - public/docs/_examples/pipes/ts/app/fetch-json.pipe.ts - public/docs/_examples/pipes/ts/app/random-pipe.component.ts - Dart version of sample code is incomplete, but did run in beta.15. - Example no longer runs, see angular/angular#8258 - public/docs/_examples/quickstart/js/package.1.json - public/docs/_examples/quickstart/ts/package.1.json - public/docs/_examples/router/ts/app/main.2.ts - public/docs/_examples/testing/ts/app/mock-router.ts - N/A for Dart. - public/docs/js/latest/_data.json - public/docs/ts/latest/_data.json - Dart equivalent updated. - public/docs/ts/latest/guide/dependency-injection.jade - No need to update since it imports the TS version. - public/docs/ts/latest/guide/pipes.jade - Not in Dart prose yet. - tools/plunker-builder/indexHtmlTranslator.js - N/A for Dart.
|
@thso, this is ready for your review. |
chalin
changed the title
| @Pipe(name: 'exponentialStrength') | ||
| @Injectable() | ||
| @Injectable() // FIXME(chalin): unnecessary? |
There was a problem hiding this comment.
The HeroService doesn't have any dependencies at the moment. Add the decorator anyway. It is a "best practice" to apply the @Injectable() decorator from the start both for consistency and for future-proofing.
From https://angular.io/docs/ts/latest/tutorial/toh-pt4.html
Event though it's not necessary, per style guide it should be added to all classes that are supposed to be injected.
There was a problem hiding this comment.
Just saw. It's a Pipe. @Pipe like @Component() and @Directive() don't need an additional @Injectable()
There was a problem hiding this comment.
That is what I thought, but I wanted to double check in case there was a nonstandard case where it might have been necessary. Thanks for confirming. @thso do you agree?
There was a problem hiding this comment.
All core pipes have the @Injectable() annotation: https://github.com/angular/angular/blob/master/modules/angular2/src/common/pipes/lowercase_pipe.ts
But I'm not convinced either that it's effectively required. Just try and build your app and see if everything still work without it :)
There was a problem hiding this comment.
Yes, I will be glad to try that out once angular/angular#8258 is fixed :). What do you suggest in the meantime? As I pointed out elsewhere, this example will need to be fully revised soon.
|
Only minor things. |
chalin
changed the title
|
@thso: given the comments above (example no longer works and needs follow-up revision), is this good to merge? If so, can you merge it? |
Updated docs and samples to beta.16.
For this first time, among other things, I payed particular attention to the changes that were made on the
tsside of things (#1178). Since I wrote it up in my notes, here is the check list oftsupdates with notes about how there might be corresponding changes on the Dart side: