Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Factory that doesn't return an object silently inject null #4575

Closed
rngadam opened this issue Oct 22, 2013 · 2 comments
Closed

Factory that doesn't return an object silently inject null #4575

rngadam opened this issue Oct 22, 2013 · 2 comments
Assignees
Milestone

Comments

@rngadam
Copy link

rngadam commented Oct 22, 2013

Factory that return null instead of an object silently inject null object:

Example:

http://plnkr.co/edit/SJM7yZafOxV0Vu6peLQL?p=preview

var app = angular.module('plunker', []);

app.factory('api', function() {
  var self = this;
  // no returns...
  // return self;
});

app.controller('MainCtrl', function($scope, api) {
  if(api) {
    $scope.name = 'api injected';
  } else {
    $scope.name = 'api not injected';
  }
});

ACTUAL: no errors on console, $scope.name set to api not injected

EXPECTED: some sort of console warning with a useful error for newbies

@btford
Copy link
Contributor

btford commented Jan 3, 2014

👍

@caitp
Copy link
Contributor

caitp commented Jan 3, 2014

We could take it a step further and throw a minErr if the factory function doesn't return something which isn't null/false/undefined, how does that sound? Ditto for the provider itself, I guess

Throwing a minErr is a pretty trivial patch to write, unfortunately using the $log service within the injector is less easy. This may not be totally possible to do without throwing. And throwing unfortunately breaks some tests (the rethrow mock provider returns no value, and I can't find where it's actually defined -_-)

shrug. I guess this could possibly break a bunch of apps anyways, so maybe it's better not to bother until 1.3

@btford btford removed the gh: issue label Aug 20, 2014
@caitp caitp self-assigned this Sep 9, 2014
caitp added a commit to caitp/angular.js that referenced this issue Sep 22, 2014
BREAKING CHANGE:

Previously, not returning a value would fail silently, and an application trying to inject the
value owuld inject an undefined value, quite possibly leading to a TypeError. Now, the application
will fail entirely, and a reason will be given.

Closes angular#4575
caitp added a commit to caitp/angular.js that referenced this issue Sep 22, 2014
BREAKING CHANGE:

Previously, not returning a value would fail silently, and an application trying to inject the
value owuld inject an undefined value, quite possibly leading to a TypeError. Now, the application
will fail entirely, and a reason will be given.

Closes angular#4575
caitp added a commit to caitp/angular.js that referenced this issue Sep 22, 2014
BREAKING CHANGE:

Previously, not returning a value would fail silently, and an application trying to inject the
value owuld inject an undefined value, quite possibly leading to a TypeError. Now, the application
will fail entirely, and a reason will be given.

Closes angular#4575
caitp added a commit to caitp/angular.js that referenced this issue Sep 22, 2014
BREAKING CHANGE:

Previously, not returning a value would fail silently, and an application trying to inject the
value owuld inject an undefined value, quite possibly leading to a TypeError. Now, the application
will fail entirely, and a reason will be given.

Closes angular#4575
@IgorMinar IgorMinar modified the milestones: 1.3.0-rc.5, 1.3.0 Oct 1, 2014
@caitp caitp closed this as completed in 0d3b69a Oct 8, 2014
bullgare pushed a commit to bullgare/angular.js that referenced this issue Oct 9, 2014
BREAKING CHANGE:

Previously, not returning a value would fail silently, and an application trying to inject the
value owuld inject an undefined value, quite possibly leading to a TypeError. Now, the application
will fail entirely, and a reason will be given.

Closes angular#4575
Closes angular#9210
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants