Dependency Injection #509

Closed
justinbmeyer opened this Issue Oct 28, 2015 · 19 comments

Projects

None yet

3 participants

@justinbmeyer
Contributor

I would like to support dependency injection with Steal. This would be great from a testing perspective. I know @phillipskevin got something like this started.

Example:

import QUnit from 'steal-qunit';

QUnit.module('Tournament Details ViewModel', {
    beforeEach: function () {
        localStorage.clear();
    }
});

test('should load a tournament', (assert) => {
    let done = assert.async(),
        localSteal,
        localTournament,
        vm;

    // clone a local copy of steal
    localSteal = steal.clone();

    // use `steal.System` to create the module to be injected for use in test
    localTournament = localSteal.System.newModule({
        __useDefault: true,
        default: {
            get: () => Promise.resolve(new can.Map({ name: 'Test Name' }))
        }
    });

    // set up local steal and call `startup()` to read configuration file
    localSteal.System.configMain = steal.System.configMain;
    localSteal.System.baseURL = steal.System.baseURL;
    localSteal.startup({ main: '@empty' }).then(function () {

        // set fully normalized module name to inject the test module
        localSteal.System.set("bitballs@0.0.1#models/tournament/tournament", localTournament);

        // re-import the module-under-test so that it will use the injected dependency
        localSteal.System.import('components/tournament/details/viewModel').then(function (ViewModel) {

            // run QUnit test using module returned by `System.import`
            vm = new ViewModel({
                tournamentId: 2
            });

            vm.bind('tournament', function (ev, newVal) {
                assert.equal(newVal.attr('name'), 'Test Name', 'with the correct name' );
                done();
            });
        });
    });
})
@phillipskevin
Contributor

I think a simplified version of this would definitely be a big win for testing with steal.

@matthewp had asked what a good API would be for doing this when I originally posted the gist of this same code. I think the keys to making this simple are:

  1. Support plain objects in place of the localTournament module being injected.
  2. Support setting with the module identifier - not the fully normalized module name.
  3. Make getting a localSteal simple.
  4. Provide synchronous APIs (there could also be async APIs, but synchronous makes it much easier at least to show in examples).

Here is one possibility of what the code above could look like with these in mind:

import { injector } from 'steal';

function tournament() {
    return Promise.resolve(new can.Map({ name: 'Test Name' }));
}

injector.set('models/tournament/tournament', tournament);

var ViewModel = injector.get('components/tournament/details/viewModel');

vm = new ViewModel({
    tournamentId: 2
});

vm.bind('tournament', function (ev, newVal) {
    assert.equal(newVal.attr('name'), 'Test Name', 'with the correct name' );
    done();
});
@justinbmeyer
Contributor

@phillipskevin that looks nice. There's two things you might consider:

  1. You are going to have to use promises to "get" a module.
  2. We could make it look/work just like a normal System object. Then we wouldn't need an aditional "injector" type, when System has pretty much everything you need already.
// clone true, true keeps all configuration and load objects
var newSystem = System.clone(true, true);
// can we make set invalidate parent modules?  if not, we'd need another method
newSystem.set('models/tournament/tournament', tournament)

// promises are a must here
injector.import('components/tournament/details/viewModel').then(function(ViewModel){
  ....
});

@matthewp what is live-reload calling under the hood to invalidate and re-import? dependency injection and hot-module swapping are super similar.

@phillipskevin
Contributor

Yeah, I was torn between sticking with the System syntax and something more like what Angular 2's DI looks like.

@phillipskevin
Contributor

We have another use case for a client project where it would be nice to be able to map a dependency to a different module in a test. Something like this would be helpful:

injector.set('models/tournament/tournament', 'test/models/tournament/tournament');

injector.get('components/tournament/details/viewModel').then(function(ViewModel){
  ....
});

We can do this with a different stealConfig for our tests, but it would be nice to not have to do that.

@matthewp
Member
matthewp commented Nov 6, 2015

You can use map config:

injector.config({ map: { "models/tournament/tournament": "test/models/tournament/tournament" } });

injector.import("components/tournament/details/viewModel").then...
@phillipskevin
Contributor

Okay, great. That's not possible now without cloning steal and doing the localSteal.startup / localSteal.System.import, right?

@justinbmeyer
Contributor

Yeah, I was torn between sticking with the System syntax and something more like what Angular 2's DI looks like.

We should stick with the System syntax. These are things that hot-module swapping also does.

Also, often, I think it's rare you want to map to another file as opposed to simply providing the module directly in the test.

@justinbmeyer
Contributor

I shouldn't say rare ... but less powerful and not that much easier.

Meaning, if I'm writing a single test, I don't want to create another file somewhere else for it. Instead I want to do:

test("something", function(){
  var tournament = {
    getList: function(){
      return new Promise()
    }
  }

  // clone true, true keeps all configuration and load objects
  var newSystem = System.clone(true, true);
  // can we make set invalidate parent modules?  if not, we'd need another method
  newSystem.set('models/tournament/tournament', tournament)

  // promises are a must here
  newSystem.import('components/tournament/details/viewModel').then(function(ViewModel){
    ....
  });

})

If I wanted to maintain tournament in another file because it's used often, presumably I could do something like:

import fakeTourney from "test/models/tournament";

test("something", function(){


  // clone true, true keeps all configuration and load objects
  var newSystem = System.clone(true, true);
  // can we make set invalidate parent modules?  if not, we'd need another method
  newSystem.set('models/tournament/tournament', fakeTourney)

  // promises are a must here
  newSystem.import('components/tournament/details/viewModel').then(function(ViewModel){
    ....
  });

})
@phillipskevin
Contributor

I think the System syntax makes sense. Would it be possible to do import { System } from 'steal'; and have it do the cloning behind the scenes?

You're right that it's not that much easier to map a module to a test module instead of just importing the test module and doing System.set.

@justinbmeyer
Contributor

You could even take that a step further and create the mocked system outside the test and use setup to pull out the importing every time:

import fakeTourney from "test/models/tournament";

var mockSystem = System.clone(true, true);
// can we make set invalidate parent modules?  if not, we'd need another method
mockSystem.set('models/tournament/tournament', fakeTourney)

module("some/module", {
  setup: function(){
    stop();
    var self = this;
    mockSystem.import('components/tournament/details/viewModel').then(function(ViewModel){
      self.ViewModel = ViewModel;
      start();
    });
  }
})

test("something", function(){
   new this.ViewModel();
   ...
})
@justinbmeyer
Contributor

Would it be possible to do import { System } from 'steal'; and have it do the cloning behind the scenes?

Something like import SystemClone from "@systemClone" might be possible. But I don't think calling .clone() is all that bad.

@phillipskevin
Contributor

Going back to the full example, here's what it would look like if you want to inject the same module for every test:

import QUnit from 'steal-qunit';
import { System } from 'steal';

function fakeTournament () {
    return Promise.resolve(new can.Map({ name: 'Test Name' }));
}

QUnit.module('Tournament Details ViewModel', {
    beforeEach(assert) {
        var done = assert.async();
        localStorage.clear();

        var SystemClone = System.clone(true, {
            'models/tournament/tournament': fakeTournament
        });

        SystemClone.import('components/tournament/details/viewModel').then((ViewModel) => {
            this.ViewModel = ViewModel;
            done();
        });
    }
});

test('should load a tournament', (assert) => {
    var done = assert.async();

    var vm = new this.ViewModel({
        tournamentId: 2
    });

    vm.bind('tournament', function (ev, newVal) {
        assert.equal(newVal.attr('name'), 'Test Name', 'with the correct name' );
        done();
    });
});

and if you wanted to inject a different module for each individual test:

import QUnit from 'steal-qunit';
import { System } from 'steal';

var SystemClone;

function fakeTournamentOne () {
    return Promise.resolve(new can.Map({ name: 'Test Name One' }));
}

function fakeTournamentTwo () {
    return Promise.resolve(new can.Map({ name: 'Test Name Two' }));
}

QUnit.module('Tournament Details ViewModel', {
    beforeEach(assert) {
        var done = assert.async();
        localStorage.clear();

        SystemClone = System.clone(true, true);
    }
});

test('should load a tournament', (assert) => {
    var done = assert.async();

    SystemClone.set('models/tournament/tournament': fakeTournamentOne);

    SystemClone.import('components/tournament/details/viewModel').then((ViewModel) => {
        var vm = new ViewModel({
            tournamentId: 2
        });

        vm.bind('tournament', function (ev, newVal) {
            assert.equal(newVal.attr('name'), 'Test Name One', 'with the correct name' );
            done();
        });
    });
});

test('should load a tournament', (assert) => {
    var done = assert.async();

    SystemClone.set('models/tournament/tournament': fakeTournamentTwo);

    SystemClone.import('components/tournament/details/viewModel').then((ViewModel) => {
        var vm = new ViewModel({
            tournamentId: 2
        });

        vm.bind('tournament', function (ev, newVal) {
            assert.equal(newVal.attr('name'), 'Test Name Two', 'with the correct name' );
            done();
        });
    });
});
@matthewp matthewp added the feature label Nov 19, 2015
@phillipskevin
Contributor

After discussing this with @matthewp, it will be useful for the "dependency injector" to be in a separate module, instead of modifying the loader directly. So, instead of import { System } from 'steal';, it would be something like import injector from 'steal-inject';.

The reason for this is that if someone wants to 'inject' a module using relative paths, the "dependency injector" would need to be context aware (discussed here). As mentioned in that issue, this is similar to what live-reload uses.

This also simplifies the API I think, because you don't need to provide an API that extends System.clone with the options needed for dependency injection. The steal-inject module would internally clone the loader and copy over its config and its loaded modules to use within the exposed set and get functions.

Here is what this would look like:

import System from 'steal-inject';

var vm;

QUnit.module('tournament', {
    beforeEach() {
        System.set('models/tournament/tournament', {
            default() {
                return Promise.resolve(new can.Map({ name: 'Test Name' }));
            }
        });

        System.get('components/tournament/details/viewModel')
            .then(ViewModel => {
                vm = new ViewModel({
                    tournamentId: 2
                });
            });
    }
});

QUnit.test('name', () => {
    vm.bind('tournament', function (ev, newVal) {
        assert.equal(newVal.attr('name'), 'Test Name', 'with the correct name' );
        done();
    });
});

@justinbmeyer - let me know if you have any feedback or want to have a discussion about this.

@matthewp
Member

Instead of returning a System I would just return a clone function. So it could look like this:

import inject from 'steal-inject';

...

QUnit.test("whatever", function(){
  inject({
    './moduleA': {
      foo: function(){ ... }
    }
  }).import("some/other/module", function(mod){

  });
});
@justinbmeyer
Contributor

@phillipskevin @matthewp I agree it should be its own module. But, I don't like how it's called "inject". This is a module that helps you clone/copy steal (maybe a system loader) which "happens" to be useful for dependency injection (but could certainly be useful to a whole host of other use cases).

I think "steal-clone" might be a better module name.

@phillipskevin
Contributor

I think steal-clone sounds good. If we want, we could even add it here as both steal-clone and steal-inject.

Can one of you create a steal-clone repo? It doesn't look like I have access to do that.

@matthewp
Member

I think it's ok to make this part of steal, add it to the ext/ folder and then add it to the config. I'd just call it steal-clone, we're only going to document one usage so no reason to add a second. People can use map if they want to rename stuff.

@phillipskevin
Contributor

Ok, sounds good.

@phillipskevin phillipskevin added this to the 0.15.0-pre milestone Mar 2, 2016
@matthewp
Member
matthewp commented Mar 4, 2016

This is being released in 0.15 as the steal-clone module: #613

@matthewp matthewp closed this Mar 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment