fix: prevent duplicate items in column selection#1503
fix: prevent duplicate items in column selection#1503ghostdevv merged 4 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe preferences provider composable now uses 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
ghostdevv
left a comment
There was a problem hiding this comment.
could you add a test for the problem you're trying to solve? if we have local storage tests possible
|
done. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
test/nuxt/composables/use-preferences-provider.spec.ts (3)
6-6: Consider importingSTORAGE_KEYfrom the source module.The storage key is hardcoded here, duplicating the value from
usePreferencesProvider.ts. If the key changes in the source, this test will silently use the wrong key and pass incorrectly.-const STORAGE_KEY = 'npmx-list-prefs' +import { STORAGE_KEY } from '../../../app/composables/usePreferencesProvider'If
STORAGE_KEYis not currently exported from the composable, consider exporting it for testability.
30-38: Assertions insideonMountedcallbacks may not propagate failures correctly.When assertions are placed inside lifecycle hooks, any thrown errors may not be caught by vitest's test runner, causing tests to pass even when assertions fail. Consider restructuring to assert after the component has mounted.
♻️ Proposed refactor using flushPromises or nextTick
+import { nextTick } from 'vue' +import { flushPromises } from '@vue/test-utils' - it('initializes with default values when storage is empty', () => { - mountWithSetup(() => { - const defaults = { theme: 'light', cols: ['name', 'version'] } - const { data } = usePreferencesProvider(defaults) - onMounted(() => { - expect(data.value).toEqual(defaults) - }) - }) + it('initializes with default values when storage is empty', async () => { + const defaults = { theme: 'light', cols: ['name', 'version'] } + let result: ReturnType<typeof usePreferencesProvider<typeof defaults>> + mountWithSetup(() => { + result = usePreferencesProvider(defaults) + }) + await flushPromises() + expect(result!.data.value).toEqual(defaults) })Apply similar changes to the other test cases (lines 40-50 and 52-62).
40-50: Test sets localStorage insidemountWithSetupcallback.The
setLocalStorage(stored)call on line 44 happens during the component'ssetup()phase, beforeonMountedhooks fire. While this ordering works, it's unconventional. Consider movingsetLocalStoragebeforemountWithSetupfor clearer test setup.♻️ Clearer test structure
it('loads values from localStorage', () => { + const stored = { theme: 'dark' } + setLocalStorage(stored) mountWithSetup(() => { const defaults = { theme: 'light' } - const stored = { theme: 'dark' } - setLocalStorage(stored) const { data } = usePreferencesProvider(defaults) onMounted(() => { expect(data.value).toEqual(stored) }) }) })Apply the same pattern to the test on lines 52-62.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
thanks merged |
fix #1474
When a configuration exists in localStorage, defu’s default merge strategy for arrays is concatenation.