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

DI issue - Injectable extending Injectable #1338

Closed
ghost opened this issue Dec 4, 2018 · 8 comments
Closed

DI issue - Injectable extending Injectable #1338

ghost opened this issue Dec 4, 2018 · 8 comments

Comments

@ghost
Copy link

ghost commented Dec 4, 2018

Hey guys,

We have around 60 dynamic modules imported into root module. These modules share most of the common behaviour / services, but few of them would like to do some overrides / customisation of common behaviour / services.

We've done it in a way where each of these dynamic modules creates own instances of common services (has it in providers), and whichever module would like to change common behaviour would extend the base service, and provide it like { provider: SomeService, useClass: MyCustomService } where MyCustomService extends SomeService. We've encountered an issue with injecting dependencies into constructors of these classes.

Actual reasoning behind this approach is quite complex, but pls check short example below.


[ ] Regression 
[ x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Both modules below are imported into one root module.

@Injectable()
export class Service {
  constructor (@Inject('Value1') value: number) {
    console.log(`Service1 -> ${value}`)
  }
}

@Injectable()
export class ServiceCustom extends Service {
  constructor (@Inject('Value2') value: number) {
    super(value)
    console.log(`Service2 -> ${value}`)
  }
}

@Module({
  providers: [
    { provide: 'Value1', useValue: 1 },
    { provide: 'Value2', useValue: 2 },
    Service
  ]
})
export class ModuleWithBaseServiceBehaviour {
}

@Module({
  providers: [
    { provide: 'Value1', useValue: 1 },
    { provide: 'Value2', useValue: 2 },
    { provide: Service, useClass: ServiceCustom }
  ]
})
export class ModuleThatOverridesBaseServiceBehaviour {
}

Both Service and ServiceCustom got injected value 2, so dependencies from Service were injected in to ServiceCustom.

Expected behavior

If I am extending an injectable class and providing this custom class in different module under same injection token, I would expect that both constructors have dependecies injected correctly.

Minimal reproduction of the problem with instructions

see example above

Environment


Nest version: 5.4.0
 
For Tooling issues:
- Node version: 10.6.0
- Platform:  Mac
@ghost ghost changed the title Injectable extending Injectable DI issue DI issue - injectable extending Injectable Dec 5, 2018
@ghost ghost changed the title DI issue - injectable extending Injectable DI issue - Injectable extending Injectable Dec 5, 2018
@kamilmysliwiec
Copy link
Member

So what is the output of this code snippet:

@Injectable()
export class Service {
  constructor (@Inject('Value1') value: number) {
    console.log(`Service1 -> ${value}`)
  }
}

@Injectable()
export class ServiceCustom extends Service {
  constructor (@Inject('Value2') value: number) {
    super(value)
    console.log(`Service2 -> ${value}`)
  }
}

This explanation:

Both Service and ServiceCustom got injected value 2, so dependencies from Service were injected in to ServiceCustom.

is not clear enough, since "2" value = Value2, not Value1

@ghost
Copy link
Author

ghost commented Dec 5, 2018

Hi Kamil,

sry that console.log might be bit misleading there as Service ctor gets also called in ServiceCustom ctor.

Anyways there should be one output line Service1 -> 1, but there is not.

Issue is that with this setup, instance of Service in first module get injected what CustomService from second modules has declared in its ctor arguments. Service gets inejcted completely wrong instances.
In our use cases it meant e.g. I did @Inject(LOGGER) and got instance of completely different thing...

@kamilmysliwiec
Copy link
Member

Are you able to create a small reproduction repository? It would help a lot

@ghost
Copy link
Author

ghost commented Dec 5, 2018

Pls check here:
https://github.com/jurajsipos/nest-di-issue

@ghost
Copy link
Author

ghost commented Dec 5, 2018

More understandable example here as it cannot resolve Service dependencies, even though it should.

@Injectable()
export class Service {
  constructor (@Inject('Value1') value: number) {
  }
}

@Injectable()
export class ServiceCustom extends Service {
  constructor (@Inject('Value2') value: number) {
    super(value)
  }
}

@Module({
  providers: [
    { provide: 'Value1', useValue: 1 },
    Service
  ]
})
export class ModuleWithBaseServiceBehaviour {
}

@Module({
  providers: [
    { provide: 'Value2', useValue: 2 },
    { provide: Service, useClass: ServiceCustom }
  ]
})
export class ModuleThatOverridesBaseServiceBehaviour {
}

@Module({
  imports: [
    ModuleWithBaseServiceBehaviour,
    ModuleThatOverridesBaseServiceBehaviour
  ]
})
export class AppModule {
}

Nest can't resolve dependencies of the Service (?). Please make sure that the argument at index [0] is available in the ModuleWithBaseServiceBehaviour context.

@kamilmysliwiec
Copy link
Member

Fixed by #1345, thanks for reporting!

@kamilmysliwiec
Copy link
Member

Fixed in 5.5.0 :)

@lock
Copy link

lock bot commented Sep 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant