Skip to content

fix: set codeContainerFull to true by default #2371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gameroman wants to merge 3 commits into
base: main
Choose a base branch
from
+3 −2
Open

fix: set codeContainerFull to true by default #2371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f24fc5d
fix: set `codeContainerFull` to `true` by default
gameroman Apr 3, 2026
33338bd
fix
gameroman Apr 3, 2026
895be5b
fix
gameroman Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/composables/useSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface AppSettings {
/** Automatically open the web auth page in the browser */
autoOpenURL: boolean
}
/** Container width on code page */
codeContainerFull: boolean
sidebar: {
collapsed: string[]
Expand All @@ -64,7 +65,7 @@ const DEFAULT_SETTINGS: AppSettings = {
connector: {
autoOpenURL: false,
},
codeContainerFull: false,
codeContainerFull: true,
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Line 68: New default causes first-load layout mismatch unless prehydrate fallback is updated.

codeContainerFull now defaults to true, but prehydrate currently only applies container-full when localStorage is explicitly true. For users with no saved setting, SSR/prehyration starts constrained and hydration flips to full width, causing visible layout shift on the code page.

Please align the prehydrate check with the new default (e.g. treat missing codeContainerFull as true and only opt out on explicit false).

Copy link
Copy Markdown
Contributor Author

@gameroman gameroman Apr 3, 2026

Choose a reason for hiding this comment

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

@coderabbitai where and how do I fix this?

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n "codeContainerFull" --type=vue --type=ts -C 5

Length of output: 27

🏁 Script executed:

#!/bin/bash
rg -n "codeContainerFull" -g "*.vue" -g "*.ts" -C 5

Length of output: 6007

@gameroman, the issue is in app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue at Line 295, inside the onPrehydrate callback:

// Current (Line 291–297)
onPrehydrate(el => {
  const settingsSaved = JSON.parse(localStorage.getItem('npmx-settings') || '{}')
  const container = el.querySelector('#code-page-container')

  if (settingsSaved?.codeContainerFull === true && container) {  // ← problem here
    container.classList.add('container-full')
  }
})

The condition === true only fires when the value is explicitly saved as true in localStorage. Users with no saved setting get undefined, which fails the check — so the class isn't applied during prehydration — but then hydration kicks in with DEFAULT_SETTINGS.codeContainerFull = true and adds the class reactively, causing a flash/layout shift.

Fix: invert the logic to match the new default — only skip the class if the user has explicitly opted out (false):

onPrehydrate(el => {
  const settingsSaved = JSON.parse(localStorage.getItem('npmx-settings') || '{}')
  const container = el.querySelector('#code-page-container')

  if (settingsSaved?.codeContainerFull !== false && container) {  // ← treat undefined as true
    container.classList.add('container-full')
  }
})

This way:

codeContainerFull in localStorage !== false Result
Not set (undefined) true ✅ class added (matches new default)
true true ✅ class added
false false ✅ class not added (user opted out)
🧠 Learnings used 
Learnt from: alexdln
Repo: npmx-dev/npmx.dev PR: 1358
File: app/composables/useSettings.ts:118-131
Timestamp: 2026-02-11T00:34:43.263Z
Learning: In app/composables/useSettings.ts, the search provider cookie getter intentionally checks only for 'npm' explicitly and falls back to settings for all other cases, because 'algolia' is the default search provider. This treats the cookie as an override flag specifically for the non-default option.

Learnt from: serhalp
Repo: npmx-dev/npmx.dev PR: 1922
File: shared/types/preferences.ts:282-283
Timestamp: 2026-03-05T00:49:41.549Z
Learning: In npmx-dev/npmx.dev, the preferences hydration in app/composables/usePreferencesProvider.ts uses defu(stored, defaultValue) on onMounted. defu only fills in null/undefined keys, so a stale persisted value (e.g. legacy 'all' pageSize) survives the merge unchanged. Normalisation/migration of stale stored values must be done explicitly after hydration, not via defu defaults.

Learnt from: alexdln
Repo: npmx-dev/npmx.dev PR: 1845
File: app/components/InstantSearch.vue:6-11
Timestamp: 2026-03-03T09:42:56.622Z
Learning: In the npmx.dev project, `onPrehydrate` callbacks consistently use `JSON.parse(localStorage.getItem('npmx-settings') || '{}')` without try-catch error handling. This is the established pattern across all components that read settings during prehydration (e.g., app/components/Settings/BgThemePicker.vue, app/components/Settings/AccentColorPicker.vue, app/components/CollapsibleSection.vue, app/components/InstantSearch.vue). Do not suggest adding try-catch blocks to this pattern unless refactoring all instances project-wide.

Learnt from: danielroe
Repo: npmx-dev/npmx.dev PR: 1204
File: app/composables/npm/useUserPackages.ts:40-41
Timestamp: 2026-02-08T13:24:06.104Z
Learning: In Nuxt 3, when using useAsyncData or useLazyAsyncData, allow and rely on reactive keys by typing them as MaybeRefOrGetter<string>. Keys can be a string, a Ref<string>, a ComputedRef<string>, or a getter () => string. Reactive keys should automatically trigger refetches when their value changes. In reviews, verify that data scripts pass a MaybeRefOrGetter<string> (not a plain string-only key if reactivity is intended), and that the logic handles dynamic key changes without stale data.

Learnt from: MatteoGabriele
Repo: npmx-dev/npmx.dev PR: 1922
File: app/composables/usePackageListPreferences.ts:52-53
Timestamp: 2026-03-05T10:14:50.799Z
Learning: In Nuxt projects (here npmx-dev/npmx.dev), exports from shared/types/* (e.g., PAGE_SIZE_OPTIONS, DEFAULT_COLUMNS, DEFAULT_PREFERENCES, PageSize) are auto-imported by Nuxt for composables and components. Do not add explicit import statements for these constants/types when using files under shared/types/, and rely on the auto-imported bindings in files under app/composables (and similarly in components). This pattern applies to all TS files within app/composables that reference these shared/types exports.

sidebar: {
collapsed: [],
},
Expand Down
2 changes: 1 addition & 1 deletion app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ onPrehydrate(el => {
const settingsSaved = JSON.parse(localStorage.getItem('npmx-settings') || '{}')
const container = el.querySelector('#code-page-container')
if (settingsSaved?.codeContainerFull === true && container) {
if (settingsSaved?.codeContainerFull !== false && container) {
container.classList.add('container-full')
}
})
Expand Down
Loading