fix(docs-theme): preserve/ensure descending version order in sidebar … #4871
Conversation
|
|
@karthik2365 is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where versioned sidebar pages did not match the order defined in _meta.ts files. The root cause was JavaScript's automatic reordering of numeric string keys in objects. The fix introduces a custom webpack loader that preserves the original key order by parsing the source file and injecting an __order__ array before the object is evaluated.
Key Changes:
- Added a new meta-loader to extract and preserve key order from
_metafiles - Modified normalize.ts to use the preserved order instead of Object.keys()
- Integrated the meta-loader into both webpack and turbopack configurations
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/nextra/src/server/meta-loader.ts | New webpack loader that parses _meta files and injects key order preservation |
| packages/nextra/src/server/page-map/normalize.ts | Updated to use preserved key order from meta-loader |
| packages/nextra/src/server/index.ts | Integrated meta-loader into webpack and turbopack configurations |
| packages/nextra/src/server/tests/meta-loader.test.ts | Comprehensive tests for key extraction logic |
| packages/nextra/package.json | Added @types/node dependency |
| packages/nextra-theme-docs/package.json | Added @types/node dependency |
| pnpm-lock.yaml | Lock file updates for dependency changes |
| package-lock.json | New lock file (4119 lines) |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (unquotedMatch && !remaining.startsWith('type:') && !remaining.startsWith('items:') && !remaining.startsWith('title:') && !remaining.startsWith('href:') && !remaining.startsWith('display:') && !remaining.startsWith('theme:')) { | ||
| // Skip common nested property names |
There was a problem hiding this comment.
The hardcoded list of property names to skip (type, items, title, href, display, theme) is fragile and could miss nested properties with the same names. Consider using a depth counter or a more robust approach to distinguish between top-level keys and nested object properties. If a top-level key happens to be named "type" or "title", it would incorrectly be skipped.
| if (unquotedMatch && !remaining.startsWith('type:') && !remaining.startsWith('items:') && !remaining.startsWith('title:') && !remaining.startsWith('href:') && !remaining.startsWith('display:') && !remaining.startsWith('theme:')) { | |
| // Skip common nested property names | |
| if (unquotedMatch) { |
|
|
||
| // Regex to match object property keys in the source | ||
| // Handles: 'key', "key", key:, 'key':, "key": | ||
| const KEY_REGEX = /(?:^|[,{]\s*)(?:'([^']+)'|"([^"]+)"|(\w[\w-]*))(?:\s*:)/gm |
There was a problem hiding this comment.
The regex pattern doesn't handle template literals or computed property names (e.g., [key]: value or `${version}`: 'title'). If _meta files use these ES6 features, the key extraction will fail silently, leading to incorrect ordering.
| for (let i = 0; i < objectContent.length; i++) { | ||
| const char = objectContent[i] | ||
|
|
||
| if (char === '{') { | ||
| depth++ | ||
| if (depth === 1) { | ||
| currentPos = i + 1 | ||
| } | ||
| } else if (char === '}') { | ||
| depth-- | ||
| } else if (depth === 1) { | ||
| // Only process at the first brace level | ||
| // Check if we're at the start of a key | ||
| const remaining = objectContent.slice(i) | ||
|
|
||
| // Match quoted key: 'key' or "key" | ||
| const quotedMatch = remaining.match(/^(['"])([^'"]+)\1\s*:/) | ||
| if (quotedMatch) { | ||
| keys.push(quotedMatch[2]!) | ||
| i += quotedMatch[0].length - 1 | ||
| continue | ||
| } | ||
|
|
||
| // Match unquoted key: key: | ||
| const unquotedMatch = remaining.match(/^([\w][\w-]*)\s*:/) | ||
| if (unquotedMatch && !remaining.startsWith('type:') && !remaining.startsWith('items:') && !remaining.startsWith('title:') && !remaining.startsWith('href:') && !remaining.startsWith('display:') && !remaining.startsWith('theme:')) { | ||
| // Skip common nested property names | ||
| keys.push(unquotedMatch[1]!) | ||
| i += unquotedMatch[0].length - 1 | ||
| continue |
There was a problem hiding this comment.
The extraction logic uses a simple character-by-character loop without handling string literals properly. If a key contains escaped quotes or the object value contains nested objects with colons, the depth tracking could become incorrect, potentially extracting wrong keys or missing legitimate top-level keys.
| for (let i = 0; i < objectContent.length; i++) { | |
| const char = objectContent[i] | |
| if (char === '{') { | |
| depth++ | |
| if (depth === 1) { | |
| currentPos = i + 1 | |
| } | |
| } else if (char === '}') { | |
| depth-- | |
| } else if (depth === 1) { | |
| // Only process at the first brace level | |
| // Check if we're at the start of a key | |
| const remaining = objectContent.slice(i) | |
| // Match quoted key: 'key' or "key" | |
| const quotedMatch = remaining.match(/^(['"])([^'"]+)\1\s*:/) | |
| if (quotedMatch) { | |
| keys.push(quotedMatch[2]!) | |
| i += quotedMatch[0].length - 1 | |
| continue | |
| } | |
| // Match unquoted key: key: | |
| const unquotedMatch = remaining.match(/^([\w][\w-]*)\s*:/) | |
| if (unquotedMatch && !remaining.startsWith('type:') && !remaining.startsWith('items:') && !remaining.startsWith('title:') && !remaining.startsWith('href:') && !remaining.startsWith('display:') && !remaining.startsWith('theme:')) { | |
| // Skip common nested property names | |
| keys.push(unquotedMatch[1]!) | |
| i += unquotedMatch[0].length - 1 | |
| continue | |
| let inString: false | "'" | '"' = false; | |
| let escape = false; | |
| for (let i = 0; i < objectContent.length; i++) { | |
| const char = objectContent[i]; | |
| if (inString) { | |
| if (escape) { | |
| escape = false; | |
| } else if (char === '\\') { | |
| escape = true; | |
| } else if (char === inString) { | |
| inString = false; | |
| } | |
| continue; | |
| } else { | |
| if (char === "'" || char === '"') { | |
| inString = char; | |
| continue; | |
| } | |
| } | |
| if (char === '{') { | |
| depth++; | |
| if (depth === 1) { | |
| currentPos = i + 1; | |
| } | |
| } else if (char === '}') { | |
| depth--; | |
| } else if (depth === 1) { | |
| // Only process at the first brace level | |
| // Check if we're at the start of a key | |
| const remaining = objectContent.slice(i); | |
| // Match quoted key: 'key' or "key" | |
| const quotedMatch = remaining.match(/^(['"])((?:\\.|[^\\'"])+)\1\s*:/); | |
| if (quotedMatch) { | |
| // Unescape the key | |
| let key = quotedMatch[2].replace(/\\(['"])/g, '$1'); | |
| keys.push(key); | |
| i += quotedMatch[0].length - 1; | |
| continue; | |
| } | |
| // Match unquoted key: key: | |
| const unquotedMatch = remaining.match(/^([\w][\w-]*)\s*:/); | |
| if (unquotedMatch && !remaining.startsWith('type:') && !remaining.startsWith('items:') && !remaining.startsWith('title:') && !remaining.startsWith('href:') && !remaining.startsWith('display:') && !remaining.startsWith('theme:')) { | |
| // Skip common nested property names | |
| keys.push(unquotedMatch[1]!); | |
| i += unquotedMatch[0].length - 1; | |
| continue; |
|
|
||
| // Regex to match object property keys in the source | ||
| // Handles: 'key', "key", key:, 'key':, "key": | ||
| const KEY_REGEX = /(?:^|[,{]\s*)(?:'([^']+)'|"([^"]+)"|(\w[\w-]*))(?:\s*:)/gm |
There was a problem hiding this comment.
Unused variable KEY_REGEX.
| const KEY_REGEX = /(?:^|[,{]\s*)(?:'([^']+)'|"([^"]+)"|(\w[\w-]*))(?:\s*:)/gm |
| let currentPos = 0 | ||
|
|
||
| for (let i = 0; i < objectContent.length; i++) { | ||
| const char = objectContent[i] | ||
|
|
||
| if (char === '{') { | ||
| depth++ | ||
| if (depth === 1) { | ||
| currentPos = i + 1 | ||
| } |
There was a problem hiding this comment.
The value assigned to currentPos here is unused.
| let currentPos = 0 | |
| for (let i = 0; i < objectContent.length; i++) { | |
| const char = objectContent[i] | |
| if (char === '{') { | |
| depth++ | |
| if (depth === 1) { | |
| currentPos = i + 1 | |
| } | |
| for (let i = 0; i < objectContent.length; i++) { | |
| const char = objectContent[i] | |
| if (char === '{') { | |
| depth++ | |
| // No need to track currentPos | |
| // if (depth === 1) { | |
| // currentPos = i + 1 | |
| // } |
1 participant
Closes #4834
Why
The sidebar ordering for versioned pages did not match the order defined in
_meta.ts.Pages listed in
_meta.tsare expected to appear in descending version order, but the generated sidebar displayed a different order due to the sorting logic.This PR fixes that inconsistency, so the sidebar correctly reflects the
_meta.tsordering (or, when applicable, uses semver-aware descending sorting).What's being changed
11.2.0,11.1.0,11.0.0appear in the correct order.Checklist