Skip to content

Fix: lazy section - #1369

Merged
vitoUwu merged 1 commit into
mainfrom
fix/lazy-section
Sep 10, 2025
Merged

vitoUwu merged 1 commit into
mainfrom
fix/lazy-section

Conversation

@vitoUwu

@vitoUwu vitoUwu commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Support forcing component rendering via URL parameters.
    • More resilient lazy loading with automatic default fallbacks when metadata is missing.
  • Bug Fixes

    • Prevent rare crashes during lazy rendering when a section is unavailable by safely skipping null sections.
    • Avoid render-time errors by guarding against missing loading fallbacks.
    • Preserve existing behavior for eager rendering paths while improving stability for deferred rendering.

@coderabbitai

coderabbitai Bot commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Updated Lazy rendering in website/sections/Rendering/Lazy.tsx to handle nullable sections, pass URL search params into shouldForceRender, use optional chaining for fallback/metadata, and early-return null when no section. Eager rendering path unchanged.

Changes

Cohort / File(s) Summary
Lazy rendering safety and control-flow tweaks
website/sections/Rendering/Lazy.tsx
- Internal type widened to allow section: Section | null during loading
- shouldForceRender now receives { ctx, searchParams } from url.searchParams
- Optional chaining for LoadingFallback and metadata; default fallback via defaultFallbackFor(resolvedSection?.metadata?.component ?? "unknown")
- Early return null when section is missing
- Eager path behavior unchanged

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant R as Request/Route
  participant L as Lazy Component
  participant LD as Loader
  participant SFR as shouldForceRender
  participant UI as UI

  R->>L: Render Lazy({ section, loading, ctx, url })
  alt loading === "eager"
    L->>UI: Render section immediately
  else loading === "lazy"
    L->>LD: load({ ctx, section })
    LD->>SFR: shouldForceRender({ ctx, searchParams: url.searchParams })
    SFR-->>LD: true/false
    alt force === true
      LD-->>L: resolvedSection (may be null)
    else
      LD-->>L: resolvedSection (deferred/conditional)
    end
    alt resolvedSection == null
      L-->>UI: null (no render)
    else resolvedSection present
      L->>UI: Render resolvedSection with fallback = resolvedSection?.LoadingFallback<br/>or defaultFallbackFor(component || "unknown")
    end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibbled through the lazy load,
A cautious bun on render road.
If section’s gone, I hop back fast—
No errors from a hasty cast.
With query crumbs to guide my sight,
I pick a fallback, soft and light. 🥕


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 08dbe34 and 0d8e02c.

📒 Files selected for processing (1)
  • website/sections/Rendering/Lazy.tsx (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Bundle & Check Apps (ubuntu-latest)
🔇 Additional comments (3)
website/sections/Rendering/Lazy.tsx (3)

101-103: Safer fallback composition: LGTM.

Optional chaining + defaultFallback covers missing LoadingFallback/metadata without blowing up.


110-113: Early-return on missing section is correct.

Prevents rendering errors when the loader yields section: null.

If nulls are expected from upstream, confirm this aligns with UX (silent no-op vs. visible placeholder).


66-69: Guard nullable section or align Props.section nullability
Casting props to { section: Section | null } lets section be null, but onBeforeResolveProps and asResolved(props.section, true) still assume non-null and will throw at runtime if it is null. Either:

  • Add a null‐check in onBeforeResolveProps before calling asResolved, e.g.:
    export const onBeforeResolveProps = (props: Props) => {
      if (!props.section) return props;
      return { ...props, section: asResolved(props.section, true) };
    };
  • Or change the Props definition to section: Section | null and update all consumers to handle null.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/lazy-section

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

@github-actions

github-actions Bot commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

Tagging Options

Should a new tag be published when this PR is merged?

  • 👍 for Patch 0.121.5 update
  • 🎉 for Minor 0.122.0 update
  • 🚀 for Major 1.0.0 update

@vitoUwu
vitoUwu merged commit 9df1139 into main Sep 10, 2025
4 of 5 checks passed
@vitoUwu
vitoUwu deleted the fix/lazy-section branch September 10, 2025 13:49
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.

2 participants