[Portal] fix portal build #7154
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
WalkthroughThe changes increase the slug uniqueness attempt limit, enable static parameter generation by exporting Changes
Sequence Diagram(s)Error: Could not generate a valid Mermaid diagram after multiple attempts. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (7)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
the
Portal
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7154 +/- ##
=======================================
Coverage 55.68% 55.68%
=======================================
Files 904 904
Lines 58340 58340
Branches 4107 4107
=======================================
Hits 32484 32484
Misses 25751 25751
Partials 105 105
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/portal/src/app/references/components/TDoc/utils/uniqueSlugger.ts (1)
5-5: LGTM - Fix addresses build issue, but consider investigating root cause.Increasing the limit from 10 to 20 helps handle more duplicate slugs during the portal build process. The change is straightforward and effective for resolving the immediate build failure.
However, consider investigating why so many duplicate slugs are being generated, as this might indicate an underlying issue with the documentation structure or slug generation logic.
You might also consider making the limit configurable rather than hardcoded:
-export function uniqueSlugger(options: { +export function uniqueSlugger(options: { base: string; isUnique: (slug: string) => boolean; + limit?: number; }) { - const limit = 20; + const limit = options.limit ?? 20; const { base, isUnique } = options;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/portal/src/app/references/components/TDoc/utils/uniqueSlugger.ts(1 hunks)apps/portal/src/app/references/typescript/[version]/[[...slug]]/page.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Lint Packages
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/portal/src/app/references/typescript/[version]/[[...slug]]/page.tsx (1)
15-15:❓ Verification inconclusive
LGTM - Enabling static generation improves performance.
Re-enabling
generateStaticParamsis a good change that will improve build-time static generation and overall performance. The coordination with the uniqueSlugger limit increase should resolve the duplicate slug issues that previously prevented this from working.Please verify that the portal build completes successfully with these changes:
🏁 Script executed:
#!/bin/bash # Description: Verify that the portal build works with static generation enabled echo "Checking if the portal app builds successfully..." cd apps/portal # Check if build script exists and run it if [ -f "package.json" ]; then if npm run build --dry-run 2>/dev/null; then echo "Build script found - you should run 'npm run build' to verify the build works" else echo "Build script not found or different command needed" fi else echo "No package.json found in portal directory" fi # Check for any build-related scripts echo "Available scripts in portal:" if [ -f "package.json" ]; then cat package.json | jq '.scripts // {}' fiLength of output: 1427
Approve static generation; verify build with pnpm
Re-enabling
generateStaticParamsis a safe, performance-boosting change now that duplicate slugs are handled by the updated uniqueSlugger. Before merging, please confirm that the portal app still builds correctly.• Location to verify:
apps/portal
• Steps:
- Install dependencies:
pnpm install- Run build:
pnpm run build
size-limit report 📦
|
joaquim-verges
force-pushed
the
05-23-fix_portal_build
branch
from
c39d7e7 to
435f1a2
Compare
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
joaquim-verges
force-pushed
the
05-23-fix_portal_build
branch
from
435f1a2 to
5c22ce8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/portal/src/app/references/components/TDoc/Summary.tsx (1)
88-89: Consider improving type safety instead of usinganycast.While the lint suppression acknowledges the complexity, casting to
anyweakens type safety. The change appears to fix a real data structure issue, but consider defining a proper type interface for the expected structure.Consider creating a type guard or interface instead of using
any:- // biome-ignore lint/suspicious/noExplicitAny: complex type - id={slugger.slug((s.children[0] as any)?.value ?? "", false)} + id={slugger.slug(getChildValue(s.children[0]) ?? "", false)}And define a helper function:
function getChildValue(child: unknown): string | undefined { if (child && typeof child === 'object' && 'value' in child) { const value = (child as { value: unknown }).value; return typeof value === 'string' ? value : undefined; } return undefined; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/portal/src/app/references/components/TDoc/Summary.tsx(1 hunks)apps/portal/src/app/references/components/TDoc/utils/getSidebarLinkgroups.ts(1 hunks)apps/portal/src/app/references/components/TDoc/utils/uniqueSlugger.ts(1 hunks)apps/portal/src/app/references/typescript/[version]/[[...slug]]/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/portal/src/app/references/components/TDoc/utils/uniqueSlugger.ts
- apps/portal/src/app/references/typescript/[version]/[[...slug]]/page.tsx
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/portal/src/app/references/components/TDoc/utils/getSidebarLinkgroups.ts (1)
413-423:✅ Verification successful
Verify the impact of changing fallback behavior from "Common" to
undefined.The change to access the
valueproperty is consistent with the similar fix inSummary.tsx, which suggests this addresses a real data structure change. However, changing the fallback from"Common"toundefinedcould have broader implications.Please verify that changing the fallback behavior doesn't break existing functionality, especially for extensions that previously defaulted to "Common". Run this script to check how this function is used:
🏁 Script executed:
#!/bin/bash # Description: Check usage of getExtensionName and how undefined return values are handled # Search for getExtensionName usage rg -A 5 -B 2 "getExtensionName" # Look for any code that expects "Common" as a default rg -A 3 -B 3 '"Common"'Length of output: 15976
getExtensionName fallback behavior remains unchanged
Verification shows that all call sites either provide their own
"Common"default or treatundefinedand"Common"identically, preserving existing behavior:
- apps/portal/src/app/typescript/v5/sidebar.tsx
UsesgetExtensionName(blockTag) || "Common", so missing values still default to"Common".- apps/portal/src/app/references/components/TDoc/Function.tsx
RendersextensionName && extensionName !== "Common" ? … : doc.name, soundefinedand"Common"both result indoc.name.- apps/portal/src/app/references/components/TDoc/utils/slugs.ts
Only truthyextensionNamevalues generate slugs; skippingundefinedmatches prior behavior.No further action required.
Summary by CodeRabbit
New Features
Improvements
PR-Codex overview
This PR primarily focuses on updating the
limitvalue inuniqueSlugger.ts, enhancing slug generation logic ingetSidebarLinkgroups.ts, and refining various documentation references across multiple files, including the addition of new hooks and restructuring existing ones.Detailed summary
limitfrom10to20inuniqueSlugger.ts.getSidebarLinkgroups.tsfor better handling of extension names.page.tsx.llms.txtfor better clarity and organization.MARKETPLACEandAIRDROPwith detailed methods inllms.txt.Summary.tsxfor slug generation.