Bug: @newInstance() becomes container default instance if it's the first instance

[krisdages]

I'm submitting a bug report

• Library Version:
  1.5.2

This is a regression from v1.4.2, where this worked correctly. I tried to implement the same test for that version, but had issues getting everything to build and run for that tag. The new TS repo structure and scripts are much appreciated. :)

Please tell us about your environment:

• Operating System:
  Linux (Ubuntu 18.04)

• Node Version:
  10.16

• NPM Version:
  N/A

• JSPM OR Webpack AND Version
  N/A

• Browser:
  all

• Language:
  all

Current behavior:
https:/krisdages/aurelia-dependency-injection/tree/bugtest/new-instance-injects-default

// test/resolver.spec.ts
// PASSES, as expected
       it('get a new instance of a dependency, without regard for existing instances in the container', () => {
        const container = new Container();
        const logger = container.get(Logger);
        const newLogger = container.get(NewInstance.of(Logger));

        expect(logger).toEqual(jasmine.any(Logger));
        expect(newLogger).toEqual(jasmine.any(Logger));
        expect(newLogger).not.toBe(logger);
      });

//FAILS
      it('new instance of a dependency does not become the default instance in the container', () => {
        const container = new Container();
//only difference is the order of the gets.
        const newLogger = container.get(NewInstance.of(Logger));
        const logger = container.get(Logger);

        expect(logger).toEqual(jasmine.any(Logger));
        expect(newLogger).toEqual(jasmine.any(Logger));
        expect(newLogger).not.toBe(logger);
      });

Expected/desired behavior:
Both tests should pass.

clone or checkout https:/krisdages/aurelia-dependency-injection

git clone https:/krisdages/aurelia-dependency-injection
git checkout bugtest/new-instance-injects-default
npm install
npm run test

• What is the expected behavior?

• What is the motivation / use case for changing the behavior?
  This is a regression from version 1.4

[bigopon]

Hmm, interesting, thanks and sorry for the wait @krisdages this' been missed.

I'm not sure if you are still interested, but I'll just comment for anyone who is

  //only difference is the order of the gets.
  const newLogger = container.get(NewInstance.of(Logger));
  const logger = container.get(Logger);

logger being the same instance with newLogger is the expected behavior in v1, where everything is automatically registered with the invoking container. It might have worked differently in 1.4.2, but it's been this way since, I'm not sure reverting it is a choice anymore. What we can do is to have a 2nd parameter on NewInstance.of() to indicate that it shouldn't result in a registration with the container, so your test will pass.

        const container = new Container();
        const newLogger = container.get(NewInstance.of(Logger, false)); // do not register
        const logger = container.get(Logger);

Though this probably will need to wait for someone who actually needs the behavior.