fix(date): support string adapter - #387
mikemikimike wants to merge 7 commits into
Conversation
Manual browser validationValidated locally with the Nuxt playground and the in-app browser:
This validates the runtime path beyond the virtual-module unit test: Nuxt dev server, Vite virtual-module resolution, Vuetify This was a manual local check and is supplementary to the automated tests; it is not a reproducible CI E2E test. |
AndreyYolkin
left a comment
There was a problem hiding this comment.
Thanks for picking this up — and for the manual browser validation write-up, that's genuinely useful. The shape of the change is right: 'string' in the DateAdapter union, a branch in buildImports/buildAdapter, and a version guard is exactly where this belongs. Two things to fix before it can land.
Blocking: the import path doesn't resolve
The generated import is:
import { StringDateAdapter } from 'vuetify/labs/date/adapters/string'That subpath resolves nowhere, on either major:
$ node -e "require.resolve('vuetify/labs/date/adapters/string')"
MODULE_NOT_FOUND # vuetify 4.1.11 and 3.12.2
$ node -e "require.resolve('vuetify/date/adapters/string')"
.../vuetify/lib/composables/date/adapters/string.js
Vuetify's exports map sends ./labs/* to ./lib/labs/*/index.js, and there is no lib/labs/date/adapters/ directory in 3.12.2 or 4.1.11. The adapter lives under the stable subpath:
import { StringDateAdapter } from 'vuetify/date/adapters/string'I suspect the neighbouring vuetify/labs/date/adapters/vuetify line is what led here — but note that branch is only reachable on Vuetify < 3.4.0. On 3.4+, buildImports returns '' for the vuetify adapter, so the labs path is never emitted. The new 'string' branch has no equivalent gate, so it emits the unresolvable path for every supported version.
Worth noting why the tests pass: they assert on the text of the generated module (expect(code).toContain(...)), not that the import resolves. Both current tests would keep passing with any string in there. Something that actually loads the adapter would be a stronger guard.
Please also reconsider passing an instance
options.adapter = new StringDateAdapter(options)Every class-shaped adapter in this module is handed to Vuetify as a class, not an instance — see options.adapter = VuetifyDateAdapter and the generic options.adapter = Adapter fallback. Vuetify then constructs it itself with { locale, formats } and keeps it in sync when the locale changes. date-fns is the single exception, and only because it needs a resolved locale object rather than a string.
StringDateAdapter's constructor is { locale: string, formats?: ... } — the same shape as VuetifyDateAdapter — so it should follow the normal path:
options.adapter = StringDateAdapterAs written, the locale comes from whatever happens to be in options at build time and won't track locale switches.
Minor
I haven't verified that 3.9.0 is the right floor — I can only confirm the adapter exists in 3.12.2 and 4.1.11. If you have a source for 3.9.0, a comment or a link would help; otherwise it's worth pinning down, since the guard turns a wrong number into a hard build error.
Not your problem
The red release check is infrastructure, not your code: pkg-pr-new publish returns 404 There is no workflow defined for pull requests from forks. The same job is simply skipped on same-repo PRs. Please ignore it.
Tagging this for the 1.0.0 milestone.
|
Addressed in commit
Validation:
The existing Nuxt Node |
Summary
Support Vuetify's built-in
StringDateAdapterthroughvuetifyOptions.date.adapter: 'string'.Changes
vuetify/date/adapters/stringimport.StringDateAdapteras a class so Vuetify owns construction and locale updates.Compatibility
Existing date adapters and their defaults are unchanged. The
stringadapter requires Vuetify 3.9.0 or newer.Tests
pnpm dev:preparepnpm -C packages/vuetify-nuxt-module test(18 files, 59 tests passed)pnpm -C packages/vuetify-nuxt-module exec vitest run test/date-configuration-plugin.test.ts(6 tests passed)pnpm -C packages/vuetify-nuxt-module lintpnpm prepackgit diff --checkA repository-wide
vue-tsc --noEmitrun still reports existing diagnostics in unrelated playground and runtime files; the changed test file is not among those diagnostics. Package lint, runtime tests, and the package build pass.Issue
Fixes #383
#383