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/mantis new cli #56

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Feat/mantis new cli #56

wants to merge 6 commits into from

Conversation

babacarbasse
Copy link
Collaborator

@babacarbasse babacarbasse commented Jun 19, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Add a "start" command to launch all generated applications simultaneously.
Implement a new "mantis" command within the CLI to facilitate the generation of shared components.
This command will streamline the process of creating reusable components that can be utilized across
multiple projects or parts of an application, enhancing modularity and reducing code duplication.
Copy link
Collaborator

@RyanClementsHax RyanClementsHax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is a pretty big rearchitecture of how templates will work. Instead of shipping them with the cli, we dynamically create the workspace and copy some files over from a different repository.

This has the benefit of not needing to ship a new version of the cli when we want to update certain files in the template, but create-nx-workspace could radically change its output or include unnecessary things that break the assumptions the template makes resulting in a broken repo.

I also see this PR being very large including the aforementioned rearchitecture, but also a new generate command. Could we split this into separate PRs? I don't want to block your work on the generate command due to discussions on the rearchitecture.

// Update tsconfig.base.json to add the new component in compilerOptions.paths array
const json = JSON.parse(content);
const COMPONENT_PATH = `shared-ui/src/lib/${name}/${name}.component`;
json.compilerOptions.paths[`@todo/${name}`] = [COMPONENT_PATH];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does @todo need to be updated to be generic to the project name?

const spinner = ora();
spinner.start('Generating component');
// generate new shared ui component
await execa('npx', [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would need to be generic depending on the package manager used.

@@ -59,6 +59,8 @@
"figlet": "^1.7.0",
"fs-extra": "^11.2.0",
"gradient-string": "^2.0.2",
"inquirer": "^9.2.22",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: why use this when we already have enquirer? I'm ok if we want to switch. We should just standardize on one though.

this.logger.error(
`Error occurred during workspace setup: ${error.message}`,
await execa(
'npx',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would need to be generic depending on the package manager selected.


export default new Command('generate')
.description('Generate a new Mantis feature')
.argument('<type>', 'The type of thing to generate')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could we list out all the different "things" that could be generated?

await startApplications(workspacePath, {
startLocalDb: isLocal,
});
.option('-m, --mongodb-uri <mongodb-uri>', 'The MongoDB URI')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for adding flags for this

import fs from 'fs-extra';
import Joi from 'joi';

export const verifyMantisProject = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you describe what the mantis.json is to be used for? It seems we can get this information elsewhere.

name: This can be found in package.json
version: This can be found in package.json by inspecting the versions of the packages installed
workspace: What is this used for? If this is to give a relative directory to the root of the mantis project, why not expect all commands to be run within the nx workspace i.e. the root?
description: This can be found in package.json
packageManager: This can be inferred using nypm

`Error occurred during workspace setup: ${error.message}`,
await execa(
'npx',
['nx', 'run-many', '--target=serve', '--projects=app-web,app-mobile'],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's standardize on the names to use. I've been using web-client and mobile-client, but I see you using different ones. Let's discuss this further. @adyngom What do you think?

await execa(
'npx',
['nx', 'run-many', '--target=serve', '--projects=app-web,app-mobile'],
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command is a little more complex than this. Could we support running the local db as well if selected?

    await execa(
      'npx',
      [
        'nx',
        'run-many',
        startLocalDb ? '--targets=serve,start-local-db' : '--target=serve',
        '--projects=web-client,mobile-client',
      ],
      { cwd: workspacePath, stdio: 'inherit' },
    );

name: 'mongoUrl',
message:
"Enter the MongoDB URI (default is 'mongodb://localhost:27017/mantis'):",
default: 'mongodb://localhost:27017/mantis',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things to note.

Could we update the prompt to be clear that we will set up the db for the user? The original prompt used the following options:

      {
        name: 'Create a local development mongo db for me (default)',
        value: 'local',
      },
      {
        name: 'Provide a mongo connection string',
        value: 'dbUrl',
      },

Secondly, can we make the db unique to the project? We wouldn't want every mantis project the user creates to connect to the same db.

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

Successfully merging this pull request may close these issues.

2 participants