Conflict between @Profile decorator and Hot Reload feature of NestJS

[manuelnucci]

The problem appears when HMR is enabled and profiles are imported. Either using the @Profile decorator and importing the class in the respective module or injecting the mapper and adding the profile manually, from time to time the hot reload fails.

The root of the problem is that, in the particular case of using the decorator, a new profile is tried to be imported and fails because there's already one prior to the hot reload. Being unable to change anything with this configuration, I tried to add the profile manually:

@Controller ('careers')
export class CareersController {
  constructor (private readonly careersService: CareersService, @InjectMapper () private readonly mapper: AutoMapper) {
    if (! mapper.getMapping (Career, ResponseCareerDto)) {// <- If profile wasn't already added
      mapper.addProfile (CareerProfile);
    }
  }

  // Rest of request handlers
}

However, the same situation happens again, receiving the error that a profile with the same source and destination was already added. This way, every time that the problem arises I must restart the whole application, which is a real pain.

Is there any way to avoid this situation, perhaps modifying some of the logic in which @Profile works?

[nartc]

Hi Manuel, sorry for all the problems you’ve run into thus far (and also grateful because now I know what to improve).

As much as I want to address these issues as quick as possible, I have been crazy busy at work this Sprint. Not sure if I have time this weekend either but I will address the issues you’ve raised next week or so. Thanks

[manuelnucci]

Hi @nartc, great to have feedback in all the requests anyway. I'll try to manage and find a workaround for now. Good luck on your week and hope the improvements help other users of the project. Talk to you later!

[nartc]

@manuelnucci The way that Hot Reload (Webpack) works is pretty black magic and also hard to modify from outside. What I can do is to allow AutoMapperModule to take in an additional option, maybe called prod, which is a boolean flag. What this flag does is it will let AutoMapper to warn about duplicate Mappings, Profiles etc... instead of throw exceptions. Maybe that'd help with app crashes because of duplications. What do you think?

[manuelnucci]

Based on HMR, I digged a bit into NestJS lifecycles, hoping to find some event that would allow me to manage profiles with more detail. On a first attemp, this is what I have:

@Controller('careers')
export class CareersController implements OnModuleInit {
  constructor(
    @InjectMapper() private readonly mapper: AutoMapper
  ) {}

  onModuleInit() {
    this.mapper.addProfile(CareerProfile);
  }
} 

However, the exception thrown persists. There's a dispose method in the mapper object which could be useful in the onModuleDestroy event, but I doesn't have the desired effect because it deletes completely all profiles, instead of only the one I want to remove.

Another approach would be to use a condition the moment I intend to add the profile, just as I exposed in the first comment. Again, that doesn't seem to do the trick neither.

So, having tried with all that (and test it with your own eyes to corroborate with what I've just said), the only clean alternative without involving a lot of lines of repetitive code is resolve the problem inside the library.

This is where we come to your proposal. If there's no other way to deal with this (because Webpack HMR is just a big black box), at least the flag would allow me to develop the application without any exception being thrown, right? Isn't any way to check if the Profile was already added and save us this warning message not adding the Profile at all?

[nartc]

The Mapper (instance of AutoMapper) does expose the profileStorage which contains ALL profiles added. It is a WeakMap<AutoMapper, MappingProfile[]>

You can get the MappingProfile[] for the mapper instance by: this.mapper.profileStorage.get(this.mapper); this would return the MappingProfile[]

[manuelnucci]

What I have right now is this:

@Controller('careers')
export class CareersController implements OnModuleInit, OnModuleDestroy {
  constructor(
    @InjectMapper() private readonly mapper: AutoMapper,
    private readonly careersService: CareersService,
  ) {}

  onModuleInit() {
    console.log('onModuleInit');
    this.mapper.addProfile(CareerProfile);
  }

  onModuleDestroy() {
    console.log('onModuleDestroy');
    // this.mapper.profileStorage.*; <-- Only 'add' and 'initialize' methods available
  }

  // Rest of request handlers
}

On each HMR, the onModuleDestroy and onModuleInit methods are called, respectively.

I couldn't find any method inside profileStorage that allowed me to get rid of the Profile already added (something like this.mapper.profileStorage.remove(CareerProfile)) and, if it would be the case, the function should also take care of removing the mapping created inside the Profile. This is the only workaround that I find feasible to manage profiles in combination with Webpack. Obviously, it requires a lot of code just to add and remove profiles in every class that makes use of them.

To sum up, do you think adding this flag is the best approach to avoid the issues with HMR? I don't have enought knowledge to propose finding a solution that involves lifecycle events, Webpack and everything else, but I think it would be great that all the functionality that I pretended to implement would work under the hood. The only downside is that, as NestJS docs state, shutdown hook listeners consume system resources, so they are disabled by default, and they would be necessary for everything to work, at least during development.

Let me know your thoughts about this and what path you consider it's best.

[nartc]

Well you're right. The profiles on profileStorage isn't exposed (it shouldn't be). The purpose of exposing the profileStorage is for the consumers to read-only the profiles.

Yeah, at this point, I believe the flag is what we need to move forward with this issue.

so something like:

AutoMapperModule.withMapper({
   prodMode: process.env.NODE_ENV === 'production'
})

so that will be false by default aka you will not see thrown exceptions by default. Only in production mode you'd see it.

[manuelnucci]

Great, I'll wait for the release then.

[nartc]

🎉 This issue has been resolved in version 3.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[nartc]

@manuelnucci use withMapper({ throwError: true/false }). This is default to true which is opposite to what I discussed in previous comments. Defaulting to true allows me to keep the current behavior of Exception Handler in @nartc/automapper in tact.

[manuelnucci]

@nartc Thank you! Everything works as expected. Very happy that you were able to implement this feature ASAP. I'll keep looking for improvements for the library!!