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

fix($injector): provider can now be defined in the array format #1611

Closed
wants to merge 1 commit into from
Closed

fix($injector): provider can now be defined in the array format #1611

wants to merge 1 commit into from

Conversation

sudhirj
Copy link
Contributor

@sudhirj sudhirj commented Nov 26, 2012

injector.instantiate is now called for arrays too, instead of only for functions.

Closes #1452

@sudhirj
Copy link
Contributor Author

sudhirj commented Nov 26, 2012

Closed #1601 and opened this instead.

@IgorMinar
Copy link
Contributor

for consistency the commit method should say $injector instead of injector

@sudhirj
Copy link
Contributor Author

sudhirj commented Nov 29, 2012

can you make the test more meaningful by introducing a dependency for Type. then you can write the test as ['dep1', Type]

Yes, I was trying that, but I wasn't able to get the injector to recognize the other dep. Will try again.

@sudhirj
Copy link
Contributor Author

sudhirj commented Nov 29, 2012

Okay, drawing a blank here... don't think I know enough about how the injector works. I've tried the following:

          expect(createInjector([function($provide){
            $provide.provider('dep1', {
              $get: valueFn('xyz')
            });
          }, function($provide) {            
            $provide.provider('value', ['dep1', Type]);
          }]).get('value')).toEqual('abc');

and

          expect(createInjector([function($provide) {
            $provide.provider('dep1', {
              $get: valueFn('xyz')
            });
            $provide.provider('value', ['dep1', Type]);
          }]).get('value')).toEqual('abc');

Both of which just give me Error: Unknown provider: dep1 from.... I remember trying other combinations too, to no avail. Help?

@pkozlowski-opensource
Copy link
Member

@sudhirj I'm not 100% sure but maybe sth like this:

it('should configure $provide using an array', function() {
  function Type(PREFIX) {
    this.prefix = PREFIX;
  };
  Type.prototype.$get = function() {
    return this.prefix + 'def';
  };
  expect(createInjector([function($provide) {
    $provide.constant('PREFIX', 'abc');
    $provide.provider('value', ['PREFIX', Type]);
  }]).get('value')).toEqual('abcdef');
});

Pushed it to my branch (https://github.com/pkozlowski-opensource/angular.js/commits/pr1611) and the tests are passing.

@IgorMinar does it make sense?

`injector.instantiate` is now called for arrays too, instead of only for functions.

Closes #1452
@sudhirj
Copy link
Contributor Author

sudhirj commented Dec 1, 2012

Don't see why $provider.constant works but not multiple calls to $provide.provider. Is .provider a single use method that overwrites the last given provider or something?

Either way, for the purposes for this issue, LGTM. I've amended the original commit - didn't think this needs two commits.

@pkozlowski-opensource
Copy link
Member

@sudhirj the thing is that a provider can only be injected with constants and other providers. So we don't have much options for this test. You've already submitted the constant-based version and here is how injecting a provider could look like:

it('should configure $provide using an array', function() {
  function Type(dep1Provider) {
    this.dep1Provider = dep1Provider;
  };
  Type.prototype.$get = function() {
    return 'abc ' + typeof this.dep1Provider;
  };
  expect(createInjector([function($provide) {
    $provide.provider('dep1', {
      $get: valueFn('abc')
    });
    $provide.provider('value', ['dep1Provider', Type]);
  }]).get('value')).toEqual('abc object');
});

But for the purpose of this fix the constant-based version is enough and is shorter. I think that your PR is in order now, to be merged.

@pkozlowski-opensource
Copy link
Member

The latest version landed as 2c405f4. Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impossible to inject providers into Module.provider() when array-style DI-annotations are used
3 participants