Skip to content

fix: prevent duplicate items in column selection#1503

Merged
ghostdevv merged 4 commits intonpmx-dev:mainfrom
rzzf:fix/1474
Feb 14, 2026
Merged

fix: prevent duplicate items in column selection#1503
ghostdevv merged 4 commits intonpmx-dev:mainfrom
rzzf:fix/1474

Conversation

@rzzf
Copy link
Contributor

@rzzf rzzf commented Feb 14, 2026

fix #1474

When a configuration exists in localStorage, defu’s default merge strategy for arrays is concatenation.

@vercel
Copy link

vercel bot commented Feb 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 14, 2026 5:00pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 14, 2026 5:00pm
npmx-lunaria Ignored Ignored Feb 14, 2026 5:00pm

Request Review

@codecov
Copy link

codecov bot commented Feb 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 14, 2026

📝 Walkthrough

Walkthrough

The preferences provider composable now uses createDefu to create a local defu reducer with a custom merge rule: when both target and incoming values are arrays, the incoming array replaces the existing array and the merge is marked handled. Hydration merges stored preferences with defaults using this reducer. A test suite for usePreferencesProvider was added, covering default initialisation, loading persisted values from localStorage, and array replacement behaviour to prevent duplicates.

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (35 files):

⚔️ .github/workflows/autofix.yml (content)
⚔️ .github/workflows/ci.yml (content)
⚔️ .github/workflows/lunaria.yml (content)
⚔️ .github/workflows/welcome.yml (content)
⚔️ app/components/Package/SkillsModal.vue (content)
⚔️ app/components/Package/WeeklyDownloadStats.vue (content)
⚔️ app/components/Tooltip/App.vue (content)
⚔️ app/components/Tooltip/Base.vue (content)
⚔️ app/composables/usePreferencesProvider.ts (content)
⚔️ docs/content/2.guide/2.keyboard-shortcuts.md (content)
⚔️ i18n/locales/ar.json (content)
⚔️ i18n/locales/bg-BG.json (content)
⚔️ i18n/locales/de-DE.json (content)
⚔️ i18n/locales/en.json (content)
⚔️ i18n/locales/es.json (content)
⚔️ i18n/locales/fr-FR.json (content)
⚔️ i18n/locales/ja-JP.json (content)
⚔️ i18n/locales/pl-PL.json (content)
⚔️ i18n/locales/zh-CN.json (content)
⚔️ i18n/schema.json (content)
⚔️ lunaria/files/ar-EG.json (content)
⚔️ lunaria/files/bg-BG.json (content)
⚔️ lunaria/files/de-DE.json (content)
⚔️ lunaria/files/en-GB.json (content)
⚔️ lunaria/files/en-US.json (content)
⚔️ lunaria/files/es-419.json (content)
⚔️ lunaria/files/es-ES.json (content)
⚔️ lunaria/files/fr-FR.json (content)
⚔️ lunaria/files/ja-JP.json (content)
⚔️ lunaria/files/pl-PL.json (content)
⚔️ lunaria/files/zh-CN.json (content)
⚔️ package.json (content)
⚔️ pnpm-lock.yaml (content)
⚔️ shared/utils/package-analysis.ts (content)
⚔️ test/unit/shared/utils/package-analysis.spec.ts (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description is directly related to the changeset, referencing issue #1474 and explaining the specific problem with array merging in localStorage.
Linked Issues check ✅ Passed The code changes successfully address issue #1474 by modifying the defu merge strategy to replace array values instead of concatenating them, preventing duplicate items in the column selection UI.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the array merging behaviour in usePreferencesProvider and adding appropriate test coverage for the fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch fix/1474
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
test/nuxt/composables/use-preferences-provider.spec.ts (1)

30-62: Consider simplifying assertion timing for clarity.

The tests use onMounted callbacks for assertions, which works but relies on Vue Test Utils executing lifecycle hooks synchronously during mount(). A slightly clearer pattern would be to await nextTick() after mount and then assert on the returned data ref directly, making the test flow more explicit:

import { nextTick } from 'vue'

it('loads values from localStorage', async () => {
  setLocalStorage({ theme: 'dark' })
  let data: Ref<{ theme: string }>
  mountWithSetup(() => {
    const result = usePreferencesProvider({ theme: 'light' })
    data = result.data
  })
  await nextTick()
  expect(data!.value).toEqual({ theme: 'dark' })
})

This is optional — the current approach is functional.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@ghostdevv ghostdevv left a comment

Choose a reason for hiding this comment

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

could you add a test for the problem you're trying to solve? if we have local storage tests possible

@rzzf
Copy link
Contributor Author

rzzf commented Feb 14, 2026

done.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (3)
test/nuxt/composables/use-preferences-provider.spec.ts (3)

6-6: Consider importing STORAGE_KEY from 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_KEY is not currently exported from the composable, consider exporting it for testability.


30-38: Assertions inside onMounted callbacks 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 inside mountWithSetup callback.

The setLocalStorage(stored) call on line 44 happens during the component's setup() phase, before onMounted hooks fire. While this ordering works, it's unconventional. Consider moving setLocalStorage before mountWithSetup for 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>
@ghostdevv ghostdevv added this pull request to the merge queue Feb 14, 2026
Merged via the queue into npmx-dev:main with commit 1aa9b6b Feb 14, 2026
17 checks passed
@rzzf
Copy link
Contributor Author

rzzf commented Feb 14, 2026

thanks merged

@rzzf rzzf deleted the fix/1474 branch February 14, 2026 17:32
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.

Columsn selection on org search show items twice

2 participants