Skip to content

fix(date): allow configure i18n when using custom adapter#265

Closed
mmnoori wants to merge 1 commit into
vuetifyjs:mainfrom
mmnoori:patch-1
Closed

fix(date): allow configure i18n when using custom adapter#265
mmnoori wants to merge 1 commit into
vuetifyjs:mainfrom
mmnoori:patch-1

Conversation

@mmnoori

@mmnoori mmnoori commented Aug 19, 2024

Copy link
Copy Markdown

If you want to use a custom date adapter, you can configure it using vuetifyOptions.date.adapter = 'custom'

If you want to use a custom date adapter, you can configure it using vuetifyOptions.date.adapter = 'custom'
@userquin

Copy link
Copy Markdown
Member

If you've a custom adapter the plugin cannot do the work for you, the user should add the corresponding logic: changes in this PR does nothing since the configuration is empty (check the virtual date module here https://github.com/vuetifyjs/nuxt-module/blob/main/src/vite/vuetify-date-configuration-plugin.ts#L34-L38 and the empty code here https://github.com/vuetifyjs/nuxt-module/blob/main/src/vite/vuetify-date-configuration-plugin.ts#L45)

@userquin

Copy link
Copy Markdown
Member

uhmm, looks like the problem is about i18n, right?

@userquin userquin changed the title Update date.ts fix(date): allow configure i18n when using custom adapter Aug 19, 2024
return
// if (adapter === 'custom' || !enabled)
// return

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't be

if (!enabled)
    return

?

@mmnoori mmnoori Aug 19, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I am using i18n.
i just know when vuetifyOptions.date.adapter is NOT set to 'custom',
configureDate plugin won't change date localization.
I assume your code is right 👍👍👍

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mmnoori can you explain the problem? changing the code with my previous suggestion will fix the problem you're trying to solve?

@AndreyYolkin AndreyYolkin added bug Something isn't working area: date Date adapter & useDate area: i18n Internationalization / locales needs rebase Has merge conflicts; needs rebase on base branch labels Jun 6, 2026
@AndreyYolkin

Copy link
Copy Markdown
Contributor

Thanks for the PR, and sorry for the long delay. Closing this one because the approach isn't safe to merge as-is.

The change comments out the whole guard:

// if (adapter === 'custom' || !enabled)
//   return

That guard does two things, and disabling both has side effects:

  • !enabled — skips date configuration entirely when the date feature is off. Removing this runs dateConfiguration() unconditionally.
  • adapter === 'custom' — the custom-adapter path is intentionally handled separately (you provide the adapter yourself via the vuetify:configuration hook), so the built-in configureDate must not run for it.

The underlying need — getting i18n locales applied when using a custom date adapter — is legitimate, but it needs a targeted fix (e.g. still applying the i18n locale map for the custom path without running the rest of the built-in configuration), not removing the guard wholesale. Note also the diff targets the pre-monorepo path (src/runtime/plugins/date.ts, now under packages/vuetify-nuxt-module/).

This whole date/config layer is being reworked in the v2 rewrite (statically-analysable vuetify.config.ts, no Vite plugins — see userquin/vuetify-nuxt-module-nuxt-v4), where custom-adapter + i18n is reconsidered. If you still hit this on the current release, please open an issue with a minimal reproduction and we'll track it. Thanks again! 🙏

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

Labels

area: date Date adapter & useDate area: i18n Internationalization / locales bug Something isn't working needs rebase Has merge conflicts; needs rebase on base branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants