Skip to content

fix(router): make wrapScreen public for RouterView compatibility (Closes #1928)#1940

Open
madhuri-perumalla wants to merge 1 commit into
Karanjot786:mainfrom
madhuri-perumalla:fix/router-view-api
Open

fix(router): make wrapScreen public for RouterView compatibility (Closes #1928)#1940
madhuri-perumalla wants to merge 1 commit into
Karanjot786:mainfrom
madhuri-perumalla:fix/router-view-api

Conversation

@madhuri-perumalla

@madhuri-perumalla madhuri-perumalla commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Description

This PR resolves the remaining API mismatch between Router and RouterView by exposing wrapScreen() as a public Router API.

Previously, RouterView relied on the private _wrapScreen() method, which caused TypeScript build failures due to access of a private member. This change introduces a public wrapScreen() method and updates internal references to use the public API.

Related Issue

Closes #1928

Which package(s)?

  • @termuijs/router

Type of Change

  • 🐛 Bug fix (type:bug)

Checklist

  • ⭐ You starred the repo.
  • Tests pass locally: bun vitest run
  • Build passes: bun run build
  • Typecheck passes: bun run typecheck
  • You read CONTRIBUTING.md.
  • Your PR title follows type: short description.
  • Widget state mutators call markDirty() (if applicable).
  • No new any types without an inline comment explaining why.
  • No unrelated refactors bundled into this PR.

GSSoC 2026 Participation

  • Yes, I am a GSSoC 2026 contributor.

Notes for the Reviewer

The autoUnmount property and direction field on NavigateEvent are already present in the current main branch. This PR focuses only on exposing wrapScreen() as a public API and updating all internal usages accordingly, providing a minimal, backward-compatible fix for the remaining API mismatch reported in Issue #1928.

Summary by CodeRabbit

  • Bug Fixes
    • Improved router screen rendering consistency across route changes, including back/forward navigation and not-found handling.
    • Updated route view behavior to use the shared screen-wrapping path, helping keep navigation transitions aligned.

@github-actions github-actions Bot added the type:bug +10 pts. Bug fix. label Jul 2, 2026
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The private Router._wrapScreen method is renamed to a public wrapScreen method with the same RouteMatch -> VNode signature. All internal call sites in router.ts and the usage in RouterView.tsx are updated to call the new public method name.

Changes

wrapScreen API rename

Layer / File(s) Summary
Rename Router method and update call sites
packages/router/src/router.ts
_wrapScreen renamed to public wrapScreen; updated in not-found handling, matched-route handling, back(), and forward().
Update RouterView consumer
packages/router/src/RouterView.tsx
State initialization now calls router.wrapScreen(router.current) instead of the private _wrapScreen.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • Karanjot786/TermUI#1848: Introduced RouterView's dependency on the _wrapScreen helper, which this PR renames and exposes publicly.

Suggested labels: level:intermediate

Suggested reviewers: Karanjot786

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The PR description follows the template and fills the required sections with clear scope, issue, package, type, checklist, and notes.
Linked Issues check ✅ Passed The change addresses the issue's core mismatch by exposing wrapScreen publicly and updating RouterView to use the public API.
Out of Scope Changes check ✅ Passed The diff is narrowly focused on exposing wrapScreen and updating its call sites, with no unrelated changes introduced.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly summarizes the main change: exposing wrapScreen for RouterView compatibility.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@madhuri-perumalla madhuri-perumalla changed the title fix(router): make wrapScreen public for RouterView compatibility (Closes ##1928) fix(router): make wrapScreen public for RouterView compatibility (Closes #1928) Jul 2, 2026
@Karanjot786

Copy link
Copy Markdown
Owner

Thanks — the wrapScreen public-API change itself is clean (RouteMatch and VNode are already public exports, no as any leak).

Issue:

  1. bench CI is failing
    Check: bench (run 28602287058) — the log shows a Node crash dump then "Performance benchmark regressed by ≥20% on at least one metric."
    Problem: something in this change (or its base) trips the perf gate, so CI isn't green.
    Fix: check whether the regression is real (re-run bench locally with bun bench / the repo's bench script); if it's a flake, push a rebase to re-trigger; if real, address the hot-path cost.

Checklist before re-requesting review:

  • bench green
  • Tests pass locally (run: bun vitest run)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug +10 pts. Bug fix.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[GSSoC] RouterView component uses non-existent Router API methods

2 participants