fix: set codeContainerFull to true by default#2371
fix: set codeContainerFull to true by default#2371gameroman wants to merge 3 commits intonpmx-dev:mainfrom
codeContainerFull to true by default#2371Conversation
|
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! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe AppSettings type is extended with a new boolean property 🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| autoOpenURL: false, | ||
| }, | ||
| codeContainerFull: false, | ||
| codeContainerFull: true, |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
@coderabbitai where and how do I fix this?
There was a problem hiding this comment.
🧩 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.
|
cc @alex-key |
|
Ironically, we even discussed which option to make the default and settled on this one. The question is only for screens around 1280-1600px. Anything smaller doesn't matter (always fullwidth), and for a wider - fullwidth container looks too huge So imo that is a matter of taste, I think before change, we should find what others think and fixate it, so as not to repeat the fate of the cursor |
Well, I do not agree. I think that on any page default container width should be a default option. |
4 participants
🔗 Linked issue
🧭 Context
I think most who look at code page often would prefer the container to have full width
📚 Description
set
codeContainerFulltotrueby default