Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(di): change the params of Provider and provide to start with "use" #4684

Closed
wants to merge 1 commit into from

Conversation

vsavkin
Copy link
Contributor

@vsavkin vsavkin commented Oct 12, 2015

No description provided.

@vicb
Copy link
Contributor

vicb commented Oct 12, 2015

I'm -1 on useExisting, too vague.

A new comer can easily think that he could use an existing class, value or factory.

class Ford extends Car {

}

provide(Car, {useExisting: Ford}); // easy error

I would use useToken.

@0x-r4bbit
Copy link
Contributor

👍 for using useToken

@@ -109,18 +109,22 @@ var NG_API = [
'Binding',
'Binding.dependencies',
'Binding.multi',
'Binding.useExisting',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are we adding this to Binding?

@IgorMinar IgorMinar 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 Oct 12, 2015
@vsavkin vsavkin added the action: merge The PR is ready for merge by the caretaker label Oct 12, 2015
@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Oct 12, 2015
@mary-poppins
Copy link

Merging PR #4684 on behalf of @vsavkin to branch presubmit-vsavkin-pr-4684.

@vsavkin vsavkin closed this in 1aeafd3 Oct 12, 2015
@IgorMinar
Copy link
Contributor

just to follow up on useExisting vs useToken, we feel that both are a bit ambiguous in different ways. let's stick to useExisting for now and if we hear see a lot of confusion about this we can consider renaming. This feature should be rarely used, so as long as the docs clearly explain what it should be used for, I don't think we should waste too much time on the best naming. There are more frequently used apis that could use our attention instead.

@0x-r4bbit
Copy link
Contributor

SG to me

On Tue, Oct 13, 2015, 5:30 AM Igor Minar notifications@github.com wrote:

just to follow up on useExisting vs useToken, we feel that both are a bit
ambiguous in different ways. let's stick to useExisting for now and if we
hear see a lot of confusion about this we can consider renaming. This
feature should be rarely used, so as long as the docs clearly explain what
it should be used for, I don't think we should waste too much time on the
best naming. There are more frequently used apis that could use our
attention instead.


Reply to this email directly or view it on GitHub
#4684 (comment).

@angular-automatic-lock-bot
Copy link

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.
Labels
action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants