Closed
Conversation
There was a problem hiding this comment.
Copilot reviewed 7 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- package.json: Language not supported
- packages/unplugin-vuetify/package.json: Language not supported
- packages/unplugin-vuetify/tsconfig.json: Language not supported
Comments suppressed due to low confidence (1)
packages/unplugin-vuetify/src/utils.ts:46
- Pushing new kebab-case attributes directly into the 'attrs' array while iterating over it may cause duplicate entries or an infinite loop; consider iterating over a shallow copy of the array or collecting new values before merging.
attrs.push(toKebabCase(attr))
There was a problem hiding this comment.
Copilot reviewed 7 out of 10 changed files in this pull request and generated 2 comments.
Files not reviewed (3)
- package.json: Language not supported
- packages/unplugin-vuetify/package.json: Language not supported
- packages/unplugin-vuetify/tsconfig.json: Language not supported
Comment on lines
+17
to
+19
| .replace(/[^a-z]/gi, '-') | ||
| .replace(/\B([A-Z])/g, '-$1') | ||
| .toLowerCase() |
There was a problem hiding this comment.
[nitpick] The toKebabCase function directly replaces all non-alphabetic characters with '-', which may result in multiple consecutive dashes. Consider adding an additional replacement to collapse repeated dashes (e.g., using .replace(/-+/g, '-')).
Suggested change
| .replace(/[^a-z]/gi, '-') | |
| .replace(/\B([A-Z])/g, '-$1') | |
| .toLowerCase() | |
| .replace(/[^a-z]/gi, '-') // Replace non-alphabetic characters with dashes | |
| .replace(/-+/g, '-') // Collapse consecutive dashes into a single dash | |
| .replace(/\B([A-Z])/g, '-$1') // Add dashes before uppercase letters | |
| .toLowerCase() // Convert the string to lowercase |
userquin
commented
Apr 11, 2025
userquin
commented
Apr 11, 2025
userquin
commented
Apr 11, 2025
userquin
commented
Apr 11, 2025
userquin
commented
Apr 11, 2025
userquin
commented
Apr 11, 2025
Member
Author
|
New @unvuetify/monorepo repository created and initial packages versions published, we'll have some more Vite/Nuxt utilities in this new repository |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR on draft, we need to add readme.md files, add another playground and prepare docs at Vuetify:
unimportpresets andunplugin-vue-componentsresolvers vuetify#21129unimportpresets andunplugin-vue-componentresolvers can still be used registering the Vite/Webpack plugin disablingauto-import, I've only tested this using Vite)This package is a modern refactor for existing codebase, doesn't replace/deprecate existing ones.
As summary (to prepare Vuetify docs later and the readme file here):
unimportpresets or theunplugin-vue-componentsresolversunimportpresets to allow (unimport-presetssubpackage export):VuetifyXXXinsteadVXXX), directives (v-vuetify-xxxinsteadv-xxxx) and composables (useVXXXor customprefix) to avoid collisions (disabled by default)transformAssetUrlswhen enabling custom Vuetify components prefix (check details below)webtypes(I'm talking with Jan-Niklas at JetBrains (WebStorm) to drop support and use Volar instead, now directives names can usevuetifyprefix => maybe we can also add the entries with the prefix)unplugin-vue-componentresolvers to allow (unplugin-vue-component-resolverssubpackage export, we need to change the name, missingsin thecomponent) for:unimportpresetsunimportpresets: won't work sinceunplugin-vue-componentsdoesn't augment the vue module properly (iirc there is a PR to fix it)Here some GH repos using Vuetify (dev) PR:
unplugin-vuetifyuserquin/vuetify-nuxt-unimport-component-prefix#1 PRWe need to replace keys in the existing transformAssetUrls entries
current transformAssetUrls map, won't work when using Vuetify component prefix