X Tutup
Skip to content

feat(compiler): Resolvers now use DI to create reflector#7762

Closed
chuckjaz wants to merge 1 commit intoangular:masterfrom
chuckjaz:resolver-parameter
Closed

feat(compiler): Resolvers now use DI to create reflector#7762
chuckjaz wants to merge 1 commit intoangular:masterfrom
chuckjaz:resolver-parameter

Conversation

@chuckjaz
Copy link
Copy Markdown
Contributor

The reflector that is used by the metadata resolvers is now a parameter that can be initialized by DI.

  • What is the current behavior? (You can also link to an open issue here)

The metadata resolvers unconditionally use reflector from 'angular2/core'.

  • What is the new behavior (if this is a feature change)?

The metadata resolvers now use the reflector that passed in as a constructor parater.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

DirectiveResolver, PipeResolver and ViewResolver now take the reflector they will use as a parameter. Code that constructs instances manually now need to provide the Reflector instance they wish to use. If reflector from 'angular/core' is passed as the parameters, they will behave identically to previous behavior.

  • Other information:

Also introduced ReflectorReader when only read-only access to the reflector is needed. The metadata resolvers only use the methods from ReflectorReader and the type the request from DI. The platform injector was updated to request the existing Reflector instance by default for ReflectorReader.

@tbosch
Copy link
Copy Markdown
Contributor

tbosch commented Mar 24, 2016

Looks good. Maybe rename the commit into Resolvers now use DI to get the reflector?

@tbosch
Copy link
Copy Markdown
Contributor

tbosch commented Mar 24, 2016

Do you also want to change Type into any in the reflector interface?

@tbosch tbosch added pr_state: LGTM action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Mar 24, 2016
@tbosch tbosch assigned chuckjaz and unassigned tbosch Mar 24, 2016
@chuckjaz
Copy link
Copy Markdown
Contributor Author

Turns out that Reflector's type parameters were already any.

@chuckjaz chuckjaz force-pushed the resolver-parameter branch 2 times, most recently from a41dee7 to b2f8803 Compare March 24, 2016 22:39
@chuckjaz chuckjaz changed the title feat(compiler): Resolvers can now use DI to create reflector feat(compiler): Resolvers now use DI to create reflector Mar 24, 2016
@chuckjaz chuckjaz force-pushed the resolver-parameter branch from b2f8803 to 894b984 Compare March 24, 2016 22:39
@chuckjaz chuckjaz assigned tbosch and unassigned chuckjaz Mar 25, 2016
@chuckjaz chuckjaz 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 Mar 25, 2016
@kara kara removed the action: merge The PR is ready for merge by the caretaker label Mar 25, 2016
Also introduced ReflectorReader when only read-only access to the reflector
is needed.
@chuckjaz chuckjaz force-pushed the resolver-parameter branch from 894b984 to 1030947 Compare March 25, 2016 21:02
@chuckjaz chuckjaz added the action: merge The PR is ready for merge by the caretaker label Mar 25, 2016
@mhevery mhevery closed this in 506f4ce Mar 28, 2016
@chuckjaz chuckjaz deleted the resolver-parameter branch March 28, 2016 19:49
@angular-automatic-lock-bot
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 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

X Tutup