Nest: Error thrown on `moduleRef.resolve()` is a breaking change

[janhartigan]

Bug Report

Current behavior

Calling moduleRef.resolve() on non-transient providers throws an error when it did not before 7.1.2.

The relevant commit is:

https://github.com/nestjs/nest/commit/439736f051384492813bed469682a48f3c8935e0

Input Code

import { Injectable } from '@nestjs/common';
import { ModuleRef } from '@nestjs/core';

import { MyClass } from './MyClass';

@Injectable()
export class Foo {
  constructor(private readonly moduleRef: ModuleRef) {}

  async build(): Promise<MyClass> {
    return await this.moduleRef.resolve(MyClass);
  }
}

Expected behavior

I understand that resolve should only be called for transient-scoped providers, but before this update, it did not throw an error when not doing so. This is a breaking change introduced in a patch version. I have full coverage in my repos, so I was able to catch this, but for anyone who is relying on Nest not to release breaking changes in minor updates, this will be a problem.

Possible Solution

Revert the change. Re-introduce it in a major version update.

[maximelkin]

I understand that resolve should only be called for transient-scoped providers

Why default scoped should not be allowed?
I want setup app global interceptors/guards, and for this I need to resolve all dependencies of them, but, since they are Scope.DEFAULT, it's not working.

[alexgarbarev]

Same problem. I understand that resolve was changed to support "only scoped" providers, to add clarity and predictability, removing ambiguity.
At same time, there are cases, when we don't know scope of provider unless runtime. For example I might design architecture, where I dispatch to providers dynamically regarderless their scope (scope would be defined by providers authors).
If that resolve method usage is wrong, then we need an alternative that would resolve provider with any scope.

Also, we need a better exception message at least, now it says "... is marked as a scoped provider ... Please, use "resolve()" instead", but

1. my providers scope set explicitly to DEFAULT, so not sure what does means "scoped" - everything is scoped then.
2. I'm using "resolve" method

As a workaround I had to set all these providers to TRANSIENT scope.

Thanks for a great framework btw.

[janhartigan]

@maximelkin The reasoning behind this makes sense given how Nest's injector works behind the scenes. The idea is that resolve is intended to be used to force-resolve a dependency chain that has some non-global-scoped providers in it. In the Nest world, these kinds of providers don't get instantiated until the a request comes in, so it makes sense that you might need an async method to do the work. get is intended to be used for globally-scoped providers.

I think a case could be made for having a method available that does either/or...it's not always easy to know when a particular provider is request scoped or not (although frequently this is a red flag for abusing the provider scopes).

[maximelkin]

@janhartigan thank you! My bad, not carefully read the docs

[MichalLytek]

I've also encountered recently this issue while upgrading the dependencies to a minor version.

The error is really confusing, because I always call .resolve() and now for the global scoped providers, it tells me:

RecipeResolver is marked as a scoped provider.
Request and transient-scoped providers can't be used in combination with "get()" method.
Please, use "resolve()" instead."

If the real intent is to allow .get() only for global scoped providers and .resolve() for transient/request scoped, the message has to be updated because I got use "resolve()" instead while using .resolve() 😄

For all the usage mentioned here, the fix is simple - read the ModulesContainer metadata and make a proper if(wrapper.isDependencyTreeStatic() && !wrapper.isTransient) to call .get(). or .resolve() properly 😉
https://github.com/MichalLytek/typegraphql-nestjs/commit/7acd23ddc284b1dafee9329398c090d97183d910

[kamilmysliwiec]

Since there's no easy way to dynamically determine the scope of a provider, I've reverted this breaking change for now (+ error message was misleading either way 😅 ). Fixed in the latest release!