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

Add an node ace add command #4296

Merged
merged 4 commits into from Feb 20, 2024
Merged

Add an node ace add command #4296

merged 4 commits into from Feb 20, 2024

Conversation

Julien-R44
Copy link
Member

@Julien-R44 Julien-R44 commented Nov 26, 2023

This PR adds the node ace add {packageName} command to the core, allowing users to install and configure a package in a single command.

With V5, we often had feedback from people complaining that X or Y package wasn't working, because they'd forgotten to type node ace configure after installing their package.

This command would avoid this. can be used like :

node ace add @adonisjs/cache

each unknown argument will be passed to the configure command. So we can do something like this :

node ace add @adonisjs/lucid --db=sqlite

if this is a dev dependency we can use the -D flag

node ace add -D adonis-sail

I've also handled the two special cases, edge and vine, which can be installed like this:

node ace add edge
node ace add vinejs

@thetutlage
Copy link
Member

I remember we had a similar command in v4 and then many folks started criticising that AdonisJS uses its own package manager vs utilizing npm or yarn.

I suspect, if we get the same feedback again.


Re: People missing to run the configure command. I think, it was more of an UI issue that anything else. In v6 docs, this is how the installation section of a package looks like.

Screenshot 2023-11-28 at 11 17 42 AM

@RomainLanz
Copy link
Member

I remember we had a similar command in v4 and then many folks started criticising that AdonisJS uses its own package manager vs utilizing npm or yarn.

I suspect, if we get the same feedback again.

I don't think we should consider this feedback. If someone's comments seem illogical or uninformed, it's not our responsibility.

Adding a node ace add command is a nice DX improvement, even for those who are aware they'll need to follow up with the configure step.

@Julien-R44
Copy link
Member Author

I also think that people are now more used to this kind of CLI commands. Some popular framework has adopted the same. for example Astro : https://docs.astro.build/en/guides/integrations-guide/#automatic-integration-setup. Vue CLI ( now deprecated ), Expo, Redwood also have something like that

@thetutlage
Copy link
Member

Cool. How do we accept and pass other CLI args to the spawned process?

@Julien-R44
Copy link
Member Author

Are you talking about the arguments to be passed on to the package manager?
In that case, I'm not sure it's necessary. Except for --save-dev, which the node ace add command accepts. If you need to install an adonis package in dev only, you can do :

node ace add --dev @adonisjs/assembler

I'll add the -D alias for this

@Julien-R44
Copy link
Member Author

Should be ready to merge

  • added -D alias for adding as a dev dependency
  • each unknown argument will be passed to the configure command. So we can do something like node ace add @adonisjs/lucid --db=sqlite.

@Julien-R44 Julien-R44 changed the base branch from next to develop February 18, 2024 19:41
@thetutlage
Copy link
Member

Looks good to me. We will need some documentation for it and maybe a blog post too, explaining why we added this command and some quick examples.

@Julien-R44
Copy link
Member Author

Should we also replace all the two-step package installation instructions with just a single node ace add in the documentation?

image

@RomainLanz
Copy link
Member

Should we also replace all the two-step package installation instructions with just a single node ace add in the documentation?

Yes, I believe it makes sense, and we made this command to avoid people having to do two commands.

@Julien-R44
Copy link
Member Author

Send a draft PR for the documentation :

adonisjs/v6-docs#44

Let's merge in the meantime

@Julien-R44 Julien-R44 merged commit e7c668c into develop Feb 20, 2024
12 checks passed
@Julien-R44 Julien-R44 deleted the feat/ace-add branch February 20, 2024 23:09
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.

None yet

3 participants