Skip to content

fix(solid-router): allow client-side-routing by link in SVG#5667

Merged
birkskyum merged 2 commits intomainfrom
link-in-svg-solid
Nov 1, 2025
Merged

fix(solid-router): allow client-side-routing by link in SVG#5667
birkskyum merged 2 commits intomainfrom
link-in-svg-solid

Conversation

@birkskyum
Copy link
Member

@birkskyum birkskyum commented Oct 28, 2025 •
edited by coderabbitai bot
Loading

This, but for solid

Closes #5632

Summary by CodeRabbit

  • New Features

    • Interactive links can now be seamlessly created and used within SVG elements with full client-side routing support, enabling interactive SVG graphics.
    • Link component enhanced with improved SVG anchor element compatibility and event handling.

  • Tests

    • Added test to verify SVG links properly navigate using client-side routing without triggering full page reloads.

@birkskyum birkskyum added this to the catch up solid to react milestone Oct 28, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 28, 2025 •
edited
Loading

Walkthrough

The PR adds SVG anchor element support to the solid-router Link component by introducing conditional SVG rendering when createLink('svg') is used, updating target attribute retrieval to use getAttribute() for compatibility with both HTML and SVG elements, and including e2e tests to verify no full-page reload occurs.

Changes

Cohort / File(s) Summary
E2E test infrastructure and app demonstration
e2e/solid-router/basic/src/main.tsx		 Added createLink import and created SVG-wrapped Link component navigating to /posts with accessible label
E2E test coverage
e2e/solid-router/basic/tests/app.spec.ts		 Added test for SVG link navigation verifying no full-page reload occurs when navigating to /posts
Link component core implementation
packages/solid-router/src/link.tsx		 Added SVG-specific rendering branch; modified click handler to read target via getAttribute('target') instead of property access for HTMLAnchorElement | SVGAElement compatibility

Sequence Diagram

sequenceDiagram
    participant User
    participant SVGLink
    participant LinkComponent
    participant Router

    User->>SVGLink: Click SVG link
    SVGLink->>LinkComponent: Click event (currentTarget = SVGAElement)
    
    rect rgba(200, 220, 255, 0.3)
    note right of LinkComponent: SVG-specific handling
    LinkComponent->>LinkComponent: getAttribute('target')<br/>instead of .target property
    LinkComponent->>LinkComponent: _asChild === 'svg'?<br/>Render SVG wrapper path
    end
    
    LinkComponent->>Router: Navigate to /posts<br/>(client-side)
    Router->>User: Update page (no full reload)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Link component logic: Review SVG-specific rendering branch and target attribute handling (getAttribute() vs. property access) to ensure correct behavior for both HTML and SVG elements
  • Test assertions: Verify DOMContentLoaded event tracking correctly detects full-page reloads and that the test accurately validates client-side navigation
  • Edge cases: Confirm that existing link behavior remains unchanged for non-SVG cases

Possibly related PRs

Suggested reviewers

  • schiller-manuel

Poem

🐰 In SVG realms where shapes reside,
A link now lives with humble pride,
No full-page reloads shall occur,
As routers navigate with silent purr, 🔗
SVG anchors, at last, set free!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix(solid-router): allow client-side-routing by link in SVG" directly and clearly describes the main change in the changeset. The summary shows modifications to the Link component and related tests specifically to enable client-side routing when Link elements are used inside SVG elements, which matches the title's intent. The title is concise, specific, and avoids vague terminology, making it immediately clear to reviewers scanning the history what the primary change addresses.
Linked Issues Check ✅ Passed The PR comprehensively addresses all coding-related objectives from linked issue #5632. The changes to packages/solid-router/src/link.tsx add SVG-specific support by modifying the handleClick method to use getAttribute('target') and implementing an SVG-specific rendering branch via createLink('svg'). Additionally, the PR includes test coverage in e2e/solid-router/basic/tests/app.spec.ts that validates client-side navigation for SVG links without triggering full page reloads. The implementation also includes example usage in e2e/solid-router/basic/src/main.tsx, demonstrating the new SVG link functionality.
Out of Scope Changes Check ✅ Passed All changes in the pull request are directly related to the stated objective of enabling client-side routing for Link elements inside SVG containers. The modifications to link.tsx implement SVG support, the new test validates the functionality, and the e2e example demonstrates usage—all contributing to the same goal. No unrelated refactoring, documentation-only changes, or fixes to separate issues are present in the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch link-in-svg-solid
📜 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 215ae1f and 199e68b.

📒 Files selected for processing (3)
  • e2e/solid-router/basic/src/main.tsx (3 hunks)
  • e2e/solid-router/basic/tests/app.spec.ts (2 hunks)
  • packages/solid-router/src/link.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript in strict mode with extensive type safety across the codebase

Files:

  • packages/solid-router/src/link.tsx
  • e2e/solid-router/basic/tests/app.spec.ts
  • e2e/solid-router/basic/src/main.tsx
packages/{react-router,solid-router}/**

📄 CodeRabbit inference engine (AGENTS.md)

Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

Files:

  • packages/solid-router/src/link.tsx
e2e/**

📄 CodeRabbit inference engine (AGENTS.md)

Store end-to-end tests under the e2e/ directory

Files:

  • e2e/solid-router/basic/tests/app.spec.ts
  • e2e/solid-router/basic/src/main.tsx
🧠 Learnings (4)
📓 Common learnings 
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
📚 Learning: 2025-09-23T17:36:12.598Z 
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

Applied to files:

  • packages/solid-router/src/link.tsx
  • e2e/solid-router/basic/src/main.tsx
📚 Learning: 2025-10-01T18:30:26.591Z 
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.

Applied to files:

  • packages/solid-router/src/link.tsx
📚 Learning: 2025-10-08T08:11:47.088Z 
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

Applied to files:

  • e2e/solid-router/basic/tests/app.spec.ts
🧬 Code graph analysis (1)
e2e/solid-router/basic/src/main.tsx (1)
packages/solid-router/src/index.tsx (2)
  • createLink (229-229)
  • Link (229-229)
🔇 Additional comments (7)
packages/solid-router/src/link.tsx (2)

284-286: LGTM: Correct handling of target attribute for SVG compatibility.

Using getAttribute('target') instead of accessing .target directly is the right approach, as SVGAElement doesn't have a .target property like HTMLAnchorElement does. The widened type annotation properly reflects support for both HTML and SVG anchor elements.

592-599: Remove nested SVG wrapper and align with React router implementation.

The nested SVG structure in solid-router differs from React router's approach. React router uses _asChild as the root component directly (line 590: React.createElement(_asChild ? _asChild : 'a', ...)), so when _asChild='svg', the SVG becomes the root element. Solid-router instead wraps the anchor inside a new SVG element, creating unnecessary nesting.

Additionally, the filtering of the class prop (line 593) is unexplained and likely unintentional—React router passes all linkProps without filtering.

Suggested fix: Return the anchor element directly in SVG namespace rather than wrapping it, and preserve the class prop handling to match React router's behavior.

⛔ Skipped due to learnings 
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
e2e/solid-router/basic/tests/app.spec.ts (2)

2-5: LGTM: Proper test setup with dynamic port handling.

The port initialization using getTestServerPort ensures the test works correctly in different environments.

53-64: LGTM: Test correctly validates SVG link client-side navigation.

The test properly verifies the core requirement:

  • Tracks full page reloads via the domcontentloaded event
  • Targets the SVG link using the aria-label selector
  • Asserts that navigation completes without triggering a full page reload

This aligns well with the PR objectives to fix client-side routing for Link components inside SVG elements.

e2e/solid-router/basic/src/main.tsx (3)

8-8: LGTM: Proper import of createLink.

The import correctly adds createLink to support SVG link creation.

32-32: LGTM: Correct usage of createLink for SVG context.

The SvgLink component is properly created using createLink('svg'), establishing a Link component specialized for SVG usage.

77-92: LGTM: Demonstrates SVG link functionality with accessibility.

The implementation correctly demonstrates the SVG link feature:

  • Wraps the SvgLink in an outer SVG container with proper dimensions
  • Includes aria-label for accessibility and test targeting
  • Places an SVG shape (rect) inside the link for visual indication

The title element (line 79) provides additional context for screen readers.

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 and usage tips.

@nx-cloud
Copy link

nx-cloud bot commented Oct 28, 2025 •
edited
Loading

View your CI Pipeline Execution ↗ for commit 199e68b

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 8m 52s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 33s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-01 04:37:44 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 28, 2025 •
edited
Loading

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5667

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5667

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@5667

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5667

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@5667

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@5667

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@5667

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@5667

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5667

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5667

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5667

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@5667

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@5667

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@5667

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@5667

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@5667

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@5667

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@5667

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5667

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@5667

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5667

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@5667

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@5667

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-ssr-query@5667

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5667

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5667

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5667

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5667

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5667

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5667

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@5667

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5667

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5667

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5667

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5667

commit: 199e68b

@brenelz brenelz marked this pull request as ready for review November 1, 2025 04:37
@birkskyum birkskyum merged commit 04f137e into main Nov 1, 2025
6 checks passed
@birkskyum birkskyum deleted the link-in-svg-solid branch November 1, 2025 09:54
roduyemi pushed a commit to roduyemi/oss-router that referenced this pull request Nov 19, 2025
@birkskyum @brenelz @roduyemi
fix(solid-router): allow creating link for client-side-routing in SVG (…
27ca5cb 
…TanStack#5667)

* add test

* fix svg links in solid router

---------

Co-authored-by: Brenley Dueck <brenleydueck@gmail.com>
naoya7076 pushed a commit to naoya7076/router that referenced this pull request Feb 15, 2026
@birkskyum @brenelz
fix(solid-router): allow creating link for client-side-routing in SVG (…
2e3dc99 
…TanStack#5667)

* add test

* fix svg links in solid router

---------

Co-authored-by: Brenley Dueck <brenleydueck@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

package: solid-router

Projects

None yet

Milestone

catch up solid to react

Development

Successfully merging this pull request may close these issues.

test(solid-router): Fix router Link component in SVG elements #5590

2 participants

@birkskyum @brenelz