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

npx nuxi module add eslint adds as main dependency and doesn't create config file #422

Closed
meirroth opened this issue May 8, 2024 · 8 comments · Fixed by #426
Closed

Comments

@meirroth
Copy link
Contributor

meirroth commented May 8, 2024

Environment

StackBlitz: https://nuxt.new/s/v3


  • Operating System: Linux
  • Node Version: v18.18.0
  • Nuxt Version: 3.11.2
  • CLI Version: 3.11.1
  • Nitro Version: 2.9.6
  • Package Manager: npm@10.2.3

Package

@nuxt/eslint

Reproduction

Open https://nuxt.new/s/v3 and run npx nuxi module add eslint
✔ See module added
❌ See dependency added as main and not as dev
❌ See missing eslint dependency
❌ See missing eslint.config.mjs

Describe the bug

The Quick Setup sections assumes the provided command will setup all for you, but it misses correctly adding dependacy under devDependencies, it missed adding eslint, and it doesn't generate eslint.config.mjs like it states it would.

Without these things, the manual setup should be faster and therefore the recommended way to get started.

If the above is an issue with nuxi or a limitation, I think we should update the docs to clarify these things. I'd be willing to open a PR if this meets the projects goals.

Additional context

No response

Logs

No response

@antfu
Copy link
Member

antfu commented May 9, 2024

See dependency added as main and not as dev

This is working as expected, as the script will add the module where nuxt is. In the starter template nuxt is put in depdendencies instead of devDeps
https://github.com/nuxt/cli/blob/b9868fedcb4545b9f84372005801ba86d4ff1b46/src/commands/module/add.ts#L64-L67

See missing eslint dependency

That's a good point. Let me see how we can improve that

See missing eslint.config.mjs

This happens in the module, you need to run nuxi prepare or nuxi dev once to have it

@meirroth
Copy link
Contributor Author

meirroth commented May 9, 2024

See dependency added as main and not as dev

This is working as expected, as the script will add the module where nuxt is. In the starter template nuxt is put in depdendencies instead of devDeps

https://github.com/nuxt/cli/blob/b9868fedcb4545b9f84372005801ba86d4ff1b46/src/commands/module/add.ts#L64-L67

Any idea why the starters have these dependencies not in dev?

See missing eslint dependency

That's a good point. Let me see how we can improve that

👍

See missing eslint.config.mjs

This happens in the module, you need to run nuxi prepare or nuxi dev once to have it

Do you think this is clear to other users or should we update documentation to mention this?

@antfu
Copy link
Member

antfu commented May 9, 2024

Any idea why the starters have these dependencies not in dev?

I don't. But in an app I guess deps and devDeps do have much difference. If you want to discuss, I suggest you to create an issue in the template repo instead.

Do you think this is clear to other users or should we update documentation to mention this?

Totally - PR welcome if you want to help on that. Thanks

@meirroth
Copy link
Contributor Author

meirroth commented May 9, 2024

I don't. But in an app I guess deps and devDeps do have much difference. If you want to discuss, I suggest you to create an issue in the template repo instead.

nuxt/starter#723

Totally - PR welcome if you want to help on that. Thanks

#426

@antfu antfu closed this as completed in #426 May 9, 2024
@meirroth
Copy link
Contributor Author

meirroth commented May 9, 2024

@antfu Thanks for merging! Want to keep this one open for the missing eslint dependency issue? Let me know if I can help.

@antfu
Copy link
Member

antfu commented May 9, 2024

I am not sure what it's the best way to do it. The nuxi command does not support custom command yet, so I guess it might be better to just document it and ask users to install?

@meirroth
Copy link
Contributor Author

meirroth commented May 9, 2024

At this point, the Quick Setup may not be "quicker" because you have to run two commands. Perhaps we should remove this section and stick with the manual setup until the cli adds this feature. Thoughts?

BTW, I recently binge-watched your 'learn.nuxt.com' stream series and learned a lot! Thank you very much 🙏.

@meirroth
Copy link
Contributor Author

@antfu bump in case you missed my comment.

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

Successfully merging a pull request may close this issue.

2 participants