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

Feat: lazy-loading-modules-like based on user input #1048

Open
1 task done
Avivbens opened this issue Sep 4, 2023 · 10 comments
Open
1 task done

Feat: lazy-loading-modules-like based on user input #1048

Avivbens opened this issue Sep 4, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@Avivbens
Copy link
Contributor

Avivbens commented Sep 4, 2023

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

Currently, all lifecycles like onModuleInit, onModuleDestroyed, etc, are been triggered just like onApplicationBootstrap.

In that way, we cannot control events to happen in some scenarios like triggering a few commands, signing requests, etc.
In addition, we're initializing the entire application, instead of just what the CLI needs to operate for this input.

Describe the solution you'd like

The ability to configure lazy loading modules, which reduce the load time of the CLI and initialize just the needed code, based on the user input to the CLI.

Teachability, documentation, adoption, migration strategy

No response

What is the motivation / use case for changing the behavior?

  • Reduce load time
  • Trigger life cycle events based on required modules (user input)
@Avivbens Avivbens added the enhancement New feature or request label Sep 4, 2023
@jmcdo29
Copy link
Owner

jmcdo29 commented Sep 4, 2023

Like any other nest application, nest will create the modules specified by the AppModule/RootModule. nest-commander's CommandFactory is a wrapper around NestFactory.createApplicationContext.

You can create a secondary CliModule that imports only the necessary modules for the commands and have only those modules and providers be created. nest-commander doesn't take care of creating any of the providers, that's so still nest's logic, so it all still depends on how you configure the nest application

@Avivbens
Copy link
Contributor Author

Avivbens commented Sep 4, 2023

By your answer, I understand that the logic of loading modules based on the user inputs are already exist. Right?

@jmcdo29
Copy link
Owner

jmcdo29 commented Sep 4, 2023

What do you mean here by "based on the user inputs"? Inputs from the person running the CLI, or inputs from the dev?

Technically, they're both possible, one just takes more work

@Avivbens
Copy link
Contributor Author

Avivbens commented Sep 4, 2023

Let's say, for example, that the user trigger the command install.

In that case, I would like to have the ability to split the install command off the other commands with a new module, which will be initialized with all its dependencies without the other modules.

input: 'my-cli install ts-node'
App:

  • install module
  • configuration module // not created in that run
  • debug module // not created in that run

With that input, just the install module will be initialized.

@jmcdo29
Copy link
Owner

jmcdo29 commented Sep 4, 2023

Technically, yes it's possible depending on how you decide to write it all up. Again, nest-commander doesn't actually do any of the module instantiation, it's just a wrapper around NestFactory.createApplicationContext and manages calling the correct command for you after creating the commander instance.

You could check for which arg was passed before calling CommandFactory and use a specific module, though that tightly binds the command names to the main which may not be desired. You could also implement the use of lazy module loading from Nest and have the commands grab what they need at the time of calling run.

I think most of this logic though is out of the hands of nest-commander. I'll think about if it could be possible to smartly change those deps, but I'm not sure if it'll be doable

@Avivbens
Copy link
Contributor Author

Avivbens commented Sep 4, 2023

I thought the same, although it'll be super convenient to just activate that by a flag.

@micalevisk
Copy link
Contributor

@Avivbens how the API to this would look like?

@Avivbens
Copy link
Contributor Author

Avivbens commented Feb 9, 2024

@Avivbens how the API to this would look like?

I thought about something like this:

2 Commands as an example

@Command({
    name: 'shell',
    options: { isDefault: false },
})
export class ShellCommand extends CommandRunner {
    constructor(
        private readonly logger: LoggerService,
    ) {
        super()
        this.logger.setContext(ShellCommand.name)
    }
}

@Command({
    name: 'install',
    options: { isDefault: false },
})
export class InstallCommand extends CommandRunner {
    constructor(
        private readonly logger: LoggerService,
    ) {
        super()
        this.logger.setContext(InstallCommand.name)
    }
}

The lazy loading module

@Module({
    providers: [LoggerService, CheckUpdateService, ShellCommand, InstallCommand],

    // new API
    isLazy: true,
})
export class ShellModule implements OnModuleInit {

    constructor(
        private readonly checkUpdateService: CheckUpdateService,
    ){ }

    onModuleInit() {
        /* would trigger by one of the following commands:
        - cli shell
        - cli install
         */
        console.log('Module has been initialized, due to triggering one of the commands of the module.')
        this.checkUpdateService.check()
    }
}

@Avivbens Avivbens closed this as completed Feb 9, 2024
@Avivbens Avivbens reopened this Feb 9, 2024
@micalevisk
Copy link
Contributor

@Avivbens introducing that flag to @Module() is not feasible to this project.

Instead, we should find a way to use what nestjs already have: the lazy loading modules features

@Avivbens
Copy link
Contributor Author

Avivbens commented Jul 1, 2024

@Avivbens introducing that flag to @Module() is not feasible to this project.

Instead, we should find a way to use what nestjs already have: the lazy loading modules features

Well, could be
The thing is that we need to parse the command and understand which module should be initialized dynamically.

We also need to understand how we can register modules based on that (best is the understand which command related to which module).

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

No branches or pull requests

3 participants