perf: add deferRender prop for viewport-based lazy highlighting

[AVGVSTVS96]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
react-shiki	Ready	Preview	Comment	Dec 5, 2025 11:10am

[changeset-bot]

⚠️ No Changeset found

Latest commit: b4d37c7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[claude]

Claude finished @AVGVSTVS96's task —— View job

---

Code Review Complete ✅

Reviewing PR #113 for code quality, best practices, and ensuring proper changeset/documentation.

Todo List

• Read repository guidelines (CLAUDE.md) - No repo-specific guidelines found
• Review changed files for code quality and architecture
• Check test coverage for new functionality
• Verify changeset requirement for package updates
• Review documentation updates for API changes
• Check for potential bugs, performance issues, security concerns
• Provide comprehensive feedback

---

Code Review Feedback

🚨 Critical Issues

1. Missing Changeset

The changeset-bot correctly identified this PR needs a changeset. Since you're adding new API surface (deferRender prop and useDeferredRender hook), this should be at least a minor version bump:

npx changeset
# Select react-shiki -> minor
# Description: "Add deferRender prop for viewport-based lazy highlighting"

2. No Test Coverage for New Feature

The new useDeferredRender hook and deferRender prop have zero test coverage. This is a significant gap for new functionality. Consider adding:

• Unit tests for useDeferredRender hook behavior
• Integration tests for deferRender prop on component
• Tests for intersection observer cleanup
• Tests for different configuration options

⚠️ Potential Issues

3. Memory Leaks Risk (use-deferred-render.ts:208-216)

The cleanup function may not properly clear all refs:

return () => {
  if (renderTimeoutRef.current) {
    clearTimeout(renderTimeoutRef.current);
  }
  if (idleCallbackRef.current) {
    cancelIdleCallbackWrapper(idleCallbackRef.current);
  }
  observer.disconnect();
};

Recommendation: Set refs to null after clearing:

return () => {
  if (renderTimeoutRef.current) {
    clearTimeout(renderTimeoutRef.current);
    renderTimeoutRef.current = null; // Add this
  }
  if (idleCallbackRef.current) {
    cancelIdleCallbackWrapper(idleCallbackRef.current);
    idleCallbackRef.current = null; // Add this
  }
  observer.disconnect();
};

4. Race Condition (use-deferred-render.ts:168-180)

The debounce logic uses takeRecords() which may not be reliable:

const records = obs.takeRecords();
const isStillInView =
  records.length === 0 ||
  (records.at(-1)?.isIntersecting ?? false);

Issue: takeRecords() returns pending records but doesn't guarantee current state. An element might have left viewport between setTimeout and the check.

Recommendation: Consider storing intersection state in a ref and updating it in the main observer callback.

5. Polyfill Implementation (use-deferred-render.ts:75-88)

The requestIdleCallback polyfill is overly simplistic:

return window.setTimeout(() => {
  callback({
    didTimeout: false,
    timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
  });
}, 1);

Issue: Always reports didTimeout: false and hardcoded 50ms remaining time.

Recommendation: Use a more robust polyfill or consider making it configurable.

✅ Positive Aspects

6. Excellent API Design

• Clean separation between hook and component logic
• Well-typed TypeScript interfaces
• Sensible defaults with good configurability
• Backward compatible (default false)

7. Good Documentation

The README updates are comprehensive and well-structured with:

• Clear usage examples for both component and hook
• Good explanation of the performance optimization strategy
• Proper API documentation table

8. Performance Considerations

• Proper use of Intersection Observer
• Smart debouncing to prevent excessive rendering
• requestIdleCallback for non-blocking renders
• SSR safety with window checks

9. Code Quality

• Consistent naming conventions
• Proper TypeScript usage
• Good separation of concerns
• Clean component integration

📋 Minor Improvements

10. Type Safety (use-deferred-render.ts:72-73)

Consider making timeout refs more type-safe:

const renderTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const idleCallbackRef = useRef<ReturnType<typeof requestIdleCallback> | null>(null);

11. Constants Organization

The constants are well-defined but could benefit from JSDoc comments explaining the reasoning behind default values.

Summary

This is a well-architected feature with excellent API design and documentation. The main blockers are:

1. Missing changeset - Required for release
2. No test coverage - Critical for reliability

The potential memory leak and race condition should be addressed before merge. Overall, this is high-quality code that follows React and performance best practices.

Recommendation: Add changeset, implement test coverage, and address the memory cleanup issue. The race condition fix is optional but recommended for robustness.

[AVGVSTVS96]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• perf: add deferRender prop for viewport-based lazy highlighting #113 👈 (View in Graphite)
• perf: improve throttling with useThrottledDebounce hook #112
• refactor: simplify useStableOptions hook to improve dependency tracking #109
• feat: Add token output format and refactor code architecture #108
• fix: correct isTextmateTheme check for multi-theme validation #115
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.