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

Injector allows binding undefined, provide function does not fail when incorrectly used #8870

Closed
1 task done
mprobst opened this issue May 26, 2016 · 2 comments · Fixed by #13544
Closed
1 task done
Labels
area: core Issues related to the framework runtime feature Issue that requests a new feature help wanted An issue that is suitable for a community contributor (based on its complexity/scope). hotlist: error messages type: bug/fix

Comments

@mprobst
Copy link
Contributor

mprobst commented May 26, 2016

  • I'm submitting a ...
  • bug report

Current behavior

It's very easy to get incorrect behaviour out of the injector, e.g. by accidentally forgetting to pass an object literal to provide. For example, the following will silently fail and bind undefined to MyFoo.

provide(MyFoo, MyFooImpl)  // should be: {useClass: MyFooImpl}

Expected/desired behavior

This should fail on two levels:

  • provide should check its arguments, and if none of the arguments are defined, given an error.
  • it should be impossible to bind undefined, and probably also impossible to bind null.

undefined is a common error (some import or property is missing, something went wrong)

Allowing to bind null means the code that gets instantiated by the injector gets a ctor parameter (field, whatever) that does not satisfy the contract the injected code expects; doing anything with it will fail. This means very hard to debug error cases, where once the app is running, much later something somewhere fails. Guice handles this by requiring bindings to be annotated as @Nullable if they should take null, but I think we might as well just disallow it; passing null values long distance through a large software system is prone to create very hard to debug error cases.

@tbosch
Copy link
Contributor

tbosch commented Jul 7, 2016

This also happens when using the provider literals, e.g. the following code is wrong as the class is declared after the provider, but this is super hard to debug right now.

let providers = [{provide: SomeClass, useClass: SomeClass}];

class SomeClass {}

/cc @vsavkin

@mhevery mhevery added area: core Issues related to the framework runtime and removed comp: core/di labels Sep 7, 2016
@vicb vicb added the help wanted An issue that is suitable for a community contributor (based on its complexity/scope). label Sep 28, 2016
@vicb vicb added the feature Issue that requests a new feature label Oct 6, 2016
@vsavkin vsavkin removed their assignment Nov 16, 2016
DzmitryShylovich pushed a commit to DzmitryShylovich/angular that referenced this issue Dec 17, 2016
DzmitryShylovich pushed a commit to DzmitryShylovich/angular that referenced this issue Dec 17, 2016
DzmitryShylovich pushed a commit to DzmitryShylovich/angular that referenced this issue Dec 17, 2016
DzmitryShylovich pushed a commit to DzmitryShylovich/angular that referenced this issue Dec 17, 2016
DzmitryShylovich pushed a commit to DzmitryShylovich/angular that referenced this issue Dec 22, 2016
@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 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime feature Issue that requests a new feature help wanted An issue that is suitable for a community contributor (based on its complexity/scope). hotlist: error messages type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants