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

HttpModule incorrectly sets Axios instance, if multiple modules imports HttpModule #2341

Closed
maksimkurb opened this issue Jun 5, 2019 · 13 comments

Comments

@maksimkurb
Copy link

commented Jun 5, 2019

Bug Report

I have multiple modules, AppModule and OtherModule.
OtherModule imports HttpModule with config A,
AppModule imports HttpModule with config B and OtherModule

Current behavior

AppModule receives HttpService from OtherModule with config A
(http://localhost:3000 returns http 500 error, connect ECONNREFUSED 127.0.0.1:80)

Input Code

Repo: https://github.com/maksimkurb/nest-httpmodule-bug

Expected behavior

AppModule should receive HttpService with config B
(http://localhost:3000 should return {ok: true})

Environment


Nest version: 6.3.0

 
For Tooling issues:
- Node version: v12.2.0  
- Platform: Windows 

Others:

kamilmysliwiec added a commit that referenced this issue Jun 6, 2019
@kamilmysliwiec

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

Fixed in 6.3.1, thanks for reporting!

@Diluka

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

When I upgrade from 6.3.0 to 6.3.1 crashed Nest can't resolve dependencies of the CacheInterceptor (?, Reflector). Please make sure that the argument at index [0] is available in the AppModule context.

@maksimkurb

This comment has been minimized.

Copy link
Author

commented Jun 11, 2019

Is CacheInterceptor's first dependency stated in providers of AppModule or in providers and exports of AppModule imports?

@Diluka

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

I use nestjs-config@^1.4.0 to config CacheModule

    CacheModule.registerAsync({
      useFactory: config => {
        return config.get('server.cache');
      },
      inject: [ConfigService],
    }),
@maksimkurb

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

Can you make a minimal reproduction repo?

@PhilipMantrov

This comment has been minimized.

Copy link

commented Jun 28, 2019

Have the same issue that @Diluka but with mongoose module init.
Problem appear from 6.3.1, works perfect in 6.3.0.

Diluka added a commit to Diluka/nest-issue-2341 that referenced this issue Jul 11, 2019
@Diluka

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

@Diluka

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

@maksimkurb @kamilmysliwiec
comment out those will start like <=6.3.0

if (isTraversing) {
const contextModuleExports = module.exports;
children = children.filter(child =>
contextModuleExports.has(child.metatype && child.metatype.name),
);
}

This is the difference between >6.3.0 and <=6.3.0
f210ebc#diff-03695d7f859ce8dbde7678750e553570R475

I am using global module

@maksimkurb

This comment has been minimized.

Copy link
Author

commented Jul 13, 2019

You are right, your sample is crashing even without ConfigModule (I've replaced CacheModule.registerAsync(...) to CacheModule.register()).
Please, open a new issue and mention this because I can't reopen it. Seems this update fixed one bug and created another

@JustDoItSascha

This comment has been minimized.

Copy link

commented Aug 26, 2019

Fixed in 6.3.1, thanks for reporting!

I'm not sure about it, but I think this is causing now another problem: When I have two modules which both register Mongoose Schemas with MongooseModule.forFeature() and Module A imports Module B, then I can't use Schemas from Module B in Module A's services / controllers.

Or do we have to export MongooseModule from now on in every module? Or does the mongoose module needs an update?

@maksimkurb

This comment has been minimized.

Copy link
Author

commented Aug 26, 2019

You have to export MongooseModule.forFeature() in Module B if you wish to use schemas from Module A

@JustDoItSascha

This comment has been minimized.

Copy link

commented Aug 27, 2019

Ah ok, thanks! And does this also work for more than two modules? When I export MongooseModule from Model A, B and C, because they need to use the schemas from one another?

And shouldn't the docs be updated, because I think there will be more people having this issue when updating to >= 6.3.1...?

@kamilmysliwiec

This comment has been minimized.

Copy link
Member

commented Aug 27, 2019

Ah ok, thanks! And does this also work for more than two modules?

Yes

And shouldn't the docs be updated, because I think there will be more people having this issue when updating to >= 6.3.1...?

Docs are up to date. It wasn't a breaking change, but a bug that has been fixed. Everyone who didn't follow the docs can face issues now.

@nestjs nestjs locked as too heated and limited conversation to collaborators Aug 27, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.