lkaric / nestjs-twilio

Injectable Twilio client for Nestjs.
https://www.npmjs.com/package/nestjs-twilio
MIT License
43 stars 11 forks source link

does not work with sub-module #36

Closed NileshPatel17 closed 2 years ago

NileshPatel17 commented 2 years ago

Bug Report

AppModule.ts

import { Module } from '@nestjs/common';
import { ConfigModule } from '@nestjs/config';
import { AppController } from './app.controller';
import { AppService } from './app.service';
import { NotificationsModule } from './notifications/notifications.module';

@Module({
  imports: [ConfigModule.forRoot(), NotificationsModule],
  controllers: [AppController],
  providers: [AppService],
})
export class AppModule {}

NotificationsModule.ts

import { Module } from '@nestjs/common';
import { NotificationsService } from './notifications.service';
import { NotificationsController } from './notifications.controller';
import { TwilioModule } from 'nestjs-twilio';
import { TwilioVerifyService } from './twilio/twilio-verify.service';

@Module({
  imports: [
    TwilioModule.forRoot({
      accountSid: process.env.TWILIO_ACCOUNT_SID,
      authToken: process.env.TWILIO_AUTH_TOKEN,
    }),
  ],
  providers: [NotificationsService, TwilioVerifyService],
  controllers: [NotificationsController],
})
export class NotificationsModule {}

Expected Behavior

i am still new to nestjs but i think, it should work with feature module. It works with root module(Appmodule.ts)

Error

When i import it to app.module, it works but when i import it to Notifications.module, it does not work.

   TwilioModule.forRoot({
      accountSid: process.env.TWILIO_ACCOUNT_SID,
      authToken: process.env.TWILIO_AUTH_TOKEN,
    }),

I get below error when i import it to NotificationsModule.ts

/app/node_modules/twilio/lib/rest/Twilio.js:138
    throw new Error('username is required');
          ^
Error: username is required
    at new Twilio (/Users/nileshp/clients/blanclabs/medx/medx-be-notifications/app/node_modules/twilio/lib/rest/Twilio.js:138:11)
    at Object.initializer [as default] (/Users/nileshp/clients/blanclabs/medx/medx-be-notifications/app/node_modules/twilio/lib/index.js:10:10)
    at createTwilioClient (/Users/nileshp/clients/blanclabs/medx/medx-be-notifications/app/node_modules/nestjs-twilio/dist/common/twilio.utils.js:29:34)
    at createTwilioProviders (/Users/nileshp/clients/blanclabs/medx/medx-be-notifications/app/node_modules/nestjs-twilio/dist/providers/twilio.provider.js:8:51)
    at Function.forRoot (/Users/nileshp/clients/blanclabs/medx/medx-be-notifications/app/node_modules/nestjs-twilio/dist/twilio-core.module.js:25:64)
    at Function.forRoot (/Users/nileshp/clients/blanclabs/medx/medx-be-notifications/app/node_modules/nestjs-twilio/dist/twilio.module.js:17:61)
    at Object.<anonymous> (/Users/nileshp/clients/blanclabs/medx/medx-be-notifications/app/src/notifications/notifications.module.ts:9:18)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)

Your Environment

NileshPatel17 commented 2 years ago

I think, I am not using it correctly.

This solved it. forRootAync solved it.

NotificationsModule.ts

 TwilioModule.forRootAsync({
      useFactory: async () => ({
        accountSid: process.env.TWILIO_ACCOUNT_SID,
        authToken: process.env.TWILIO_AUTH_TOKEN,
      }),
    }),
lkaric commented 2 years ago

@NileshPatel17 Considering you're using @nestjs/config I'd advise you to take a look at the forRootAsync implementation in the docs, you can better utilize the use of ConfigModule in that particular case. Anyway I'll be taking a look at the synchronous implementation in the next few weeks

NileshPatel17 commented 2 years ago

@rejvban with forRootASync, it did work. but i wonder if it should work with forRoot as well. and, it has nothing to do with @nestjs/config.

TwilioModule.forRootAsync({
      useFactory: async () => ({
        accountSid: process.env.TWILIO_ACCOUNT_SID,
        authToken: process.env.TWILIO_AUTH_TOKEN,
      }),
    }),

please let me know your view.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.