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

[Refactoring] Prefer importing modules instead of including Services #523

Open
igoychev opened this issue Jul 27, 2023 · 0 comments
Open
Labels

Comments

@igoychev
Copy link
Contributor

igoychev commented Jul 27, 2023

Which area(s) of Podkrepi.bg are affected?

Development Velocity

Describe the Bug

Currently when describing jest tests we have to include all the cascading dependencies of the services in the providers section, which is not good. A better approach is to import the module.ts that contains the service, which should have already the dependencies imported properly using module import section.

To Reproduce

Illustrative example of how it is incorrectly done in the Donations.controller.spec.ts

note the long providers:[] section with specifics coming from nested dependencies

beforeEach(async () => {
    const module: TestingModule = await Test.createTestingModule({
      imports: [NotificationModule],
      controllers: [DonationsController],
      providers: [
        {
          provide: ConfigService,
          useValue: {
            get: jest.fn(),
          },
        },
        CampaignService,
        DonationsService,
        {
          provide: VaultService,
          useValue: vaultMock,
        },
        MockPrismaService,
        {
          provide: STRIPE_CLIENT_TOKEN,
          useValue: stripeMock,
        },
        PersonService,
        ExportService,
        { provide: CACHE_MANAGER, useValue: {} },
      ],
    }).compile()

Corrected example:

with almost nothing in the providers section

  beforeEach(async () => {
    const module: TestingModule = await Test.createTestingModule({
      imports: [NotificationModule, DonationsModule],
      controllers: [DonationsController],
      providers: [DonationsService],
    }).overrideProvider(PrismaService)
      .useValue(MockPrismaService)
      .compile()

Expected Behavior

To fix the dependency mess there is a need to refactor the imports and providers sections of the current modules.

Example of how it is wrongly done in the Donations module

Note how the providers:[] section contains many more services than the Donation Module actually contains.

@Module({
  imports: [
    StripeModule.forRootAsync(StripeModule, {
      inject: [ConfigService],
      useFactory: StripeConfigFactory.useFactory,
    }),
    VaultModule,
    CampaignModule,
    PersonModule,
    HttpModule,
    ExportModule,
    NotificationModule,
  ],
  controllers: [DonationsController],
  providers: [
    DonationsService,
    StripePaymentService,
    CampaignService,
    RecurringDonationService,
    PrismaService,
    VaultService,
    PersonService,
    ExportService,
  ],
  exports: [DonationsService],
})
export class DonationsModule {}

while the correct way should be like this

where the imported modules section contains only direct dependencies, so that we let NestJS to solve the nested dependencies

@Module({
  imports: [
    StripeModule.forRootAsync(StripeModule, {
      inject: [ConfigService],
      useFactory: StripeConfigFactory.useFactory,
    }),
    VaultModule,
    CampaignModule,
    PersonModule,
    HttpModule,
    ExportModule,
    NotificationModule,
    RecurringDonationModule
  ],
  controllers: [DonationsController],
  providers: [DonationsService, StripePaymentService, PrismaService],
  exports: [DonationsService],
})
export class DonationsModule {}

Which browser are you using? (if relevant)

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant