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

chore: refactor module #3117

Merged
merged 4 commits into from
Sep 21, 2024
Merged

Conversation

userquin
Copy link
Contributor

@userquin userquin commented Sep 20, 2024

🔗 Linked issue

This PR just moves logic from module.ts to the corresponding module: for example, when configuring nitro, the current module.ts will build the AdditionalSetupNitroParams (being used or not), with the new approach, the nitro.ts module will build the AdditionalSetupNitroParams when required.

The idea is to have the module.ts to orchestrate the I18n configuration: any shared data should be on the context, and the modules should do their work. The module.ts will call the initialization sequence correctly, and modules preparing shared data should add their stuff to the context (check prepare/locale-info.ts and prepare/runtime.ts).

Any module should only accept the context and nuxt as parameters: using destructuring on the context to get used data.

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

📝 Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

@userquin userquin marked this pull request as ready for review September 20, 2024 14:13
@BobbieGoede
Copy link
Collaborator

approving to run ci


ctx.genTemplate = genTemplate

nuxt.options.runtimeConfig.public.i18n.locales = simplifyLocaleOptions(nuxt, defu({}, options))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm guessing we can't move this to prepare/runtime-config.ts right? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, we can do what we want here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can move runtime config after runtime and then we can move it: do you want me to move it?

Copy link
Collaborator

@kazupon kazupon left a comment

Choose a reason for hiding this comment

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

This PR is amazing!
Thanks!

@kazupon kazupon merged commit 3f9e018 into nuxt-modules:main Sep 21, 2024
8 checks passed
@userquin userquin deleted the include-i18n-nuxt-context branch September 21, 2024 07:57
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.

3 participants