Skip to content

refactor(router): RouteData as a type#4428

Closed
cexbrayat wants to merge 1 commit into
angular:masterfrom
cexbrayat:feat/route-params-as-service
Closed

refactor(router): RouteData as a type#4428
cexbrayat wants to merge 1 commit into
angular:masterfrom
cexbrayat:feat/route-params-as-service

Conversation

@cexbrayat

@cexbrayat cexbrayat commented Sep 30, 2015

Copy link
Copy Markdown
Member

BREAKING CHANGE

The ROUTE_DATA token has been removed and replaced with a type RouteData,
allowing a type injection like we do with RouteParams.

Before:

constructor(routeParams: RouteParams, @Inject(ROUTE_DATA) routeData) {
  let id = routeParams.get('id');
  let name = ROUTE_DATA.name;
}

After:

constructor(routeParams: RouteParams, routeData: RouteData) {
  let id = routeParams.get('id');
  let name = routeData.get('name');
}

Fixes #4392

@cexbrayat cexbrayat mentioned this pull request Sep 30, 2015
Closed
@cexbrayat cexbrayat force-pushed the feat/route-params-as-service branch from 76a995d to 7e7dcc5 Compare September 30, 2015 13:54
@btford

btford commented Oct 9, 2015

Copy link
Copy Markdown
Contributor

Looks good, except two things:

  • Can you cache the RouteData object on a private field in the instruction so that instruction.routeData == instruction.routeData?
  • Can you make it a getter?

Fix those, rebase, squash, and this is good to merge 👍.

Thanks for your help, @cexbrayat!

@cexbrayat cexbrayat force-pushed the feat/route-params-as-service branch 10 times, most recently from f67608c to d9c243a Compare October 9, 2015 09:08
@cexbrayat
refactor(router): RouteData as a type
bc7bac9 
BREAKING CHANGE

The ROUTE_DATA token has been removed and replaced with a type RouteData,
allowing a type injection like we do with RouteParams.

Before:

    constructor(routeParams: RouteParams, @Inject(ROUTE_DATA) routeData) {
      let id = routeParams.get('id');
      let name = ROUTE_DATA.name;
    }

After:

    constructor(routeParams: RouteParams, routeData: RouteData) {
      let id = routeParams.get('id');
      let name = routeData.get('name');
    }

Fixes angular#4392
@cexbrayat cexbrayat force-pushed the feat/route-params-as-service branch from d9c243a to bc7bac9 Compare October 9, 2015 09:18
@cexbrayat

cexbrayat commented Oct 9, 2015

Copy link
Copy Markdown
Member Author

@btford Fixed, rebased and squashed.
_routeData is now a private field on ComponentInstruction_ and routeData() a getter.
I hope that was what you were expecting.

I sadly can't figure out how to make the Dart build pass...
Maybe you have an advice?
It looks like the default parameter value for the RouteData constructor is not correct for Dart...

@btford

btford commented Oct 9, 2015

Copy link
Copy Markdown
Contributor

I sadly can't figure out how to make the Dart build pass

No worries, I can take it from here. Thanks so much!

@naomiblack naomiblack added this to the alpha-45 milestone Oct 27, 2015
@btford btford closed this in b87da8f Oct 27, 2015
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 7, 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.

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

Reviewers

No reviews

Assignees

@btford btford

Labels

cla: yes

Projects

None yet

Milestone

beta.0

Development

Successfully merging this pull request may close these issues.

[router]: using @inject for ROUTE_DATA

4 participants

@cexbrayat @btford @naomiblack @googlebot