Skip to content

Commit 2c8d5ce

Browse files
leesb971204leesb971204
authored
fix(router): respect target attribute on inner component (#5115)
1 parent d49b204 commit 2c8d5ce

File tree

4 files changed

+234
-2
lines changed

4 files changed

+234
-2
lines changed

‎packages/react-router/src/link.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,11 +218,15 @@ export function useLinkProps<
218218

219219
// The click handler
220220
const handleClick = (e: React.MouseEvent) => {
221+
// Check actual element's target attribute as fallback
222+
const elementTarget = (e.currentTarget as HTMLAnchorElement).target
223+
const effectiveTarget = target !== undefined ? target : elementTarget
224+
221225
if (
222226
!disabled &&
223227
!isCtrlEvent(e) &&
224228
!e.defaultPrevented &&
225-
(!target || target === '_self') &&
229+
(!effectiveTarget || effectiveTarget === '_self') &&
226230
e.button === 0
227231
) {
228232
e.preventDefault()

‎packages/react-router/tests/link.test.tsx‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4683,6 +4683,116 @@ describe('createLink', () => {
46834683
const button3 = await screen.findByText('active: no - foo: no - Button3')
46844684
expect(button3.getAttribute('overrideMeIfYouWant')).toBe('Button3')
46854685
})
4686+
4687+
it('should respect target attribute set by custom component', async () => {
4688+
const CustomLinkWithTarget = React.forwardRef<
4689+
HTMLAnchorElement,
4690+
{ href?: string; children?: React.ReactNode }
4691+
>((props, ref) => (
4692+
<a ref={ref} {...props} target="_blank" rel="noopener noreferrer" />
4693+
))
4694+
4695+
const CreatedCustomLink = createLink(CustomLinkWithTarget)
4696+
4697+
const rootRoute = createRootRoute()
4698+
const indexRoute = createRoute({
4699+
getParentRoute: () => rootRoute,
4700+
path: '/',
4701+
component: () => (
4702+
<>
4703+
<h1>Index</h1>
4704+
<CreatedCustomLink to="/posts">
4705+
Posts (should open in new tab)
4706+
</CreatedCustomLink>
4707+
</>
4708+
),
4709+
})
4710+
4711+
const postsRoute = createRoute({
4712+
getParentRoute: () => rootRoute,
4713+
path: '/posts',
4714+
component: () => <h1 data-testid="posts-heading">Posts</h1>,
4715+
})
4716+
4717+
const router = createRouter({
4718+
routeTree: rootRoute.addChildren([indexRoute, postsRoute]),
4719+
history: createMemoryHistory({ initialEntries: ['/'] }),
4720+
})
4721+
4722+
const originalOpen = window.open
4723+
const openMock = vi.fn()
4724+
window.open = openMock
4725+
4726+
render(<RouterProvider router={router} />)
4727+
4728+
const postsLink = await screen.findByRole('link', {
4729+
name: 'Posts (should open in new tab)',
4730+
})
4731+
4732+
expect(postsLink).toHaveAttribute('target', '_blank')
4733+
expect(postsLink).toHaveAttribute('rel', 'noopener noreferrer')
4734+
4735+
fireEvent.click(postsLink)
4736+
4737+
await waitFor(() => {
4738+
expect(router.state.location.pathname).toBe('/')
4739+
})
4740+
4741+
await expect(screen.findByTestId('posts-heading')).rejects.toThrow()
4742+
4743+
window.open = originalOpen
4744+
})
4745+
4746+
it('should allow override of target prop even when custom component sets it', async () => {
4747+
const CustomLinkWithDefaultTarget = React.forwardRef<
4748+
HTMLAnchorElement,
4749+
{ href?: string; children?: React.ReactNode; target?: string }
4750+
>((props, ref) => <a ref={ref} target="_blank" {...props} />)
4751+
4752+
const CreatedCustomLink = createLink(CustomLinkWithDefaultTarget)
4753+
4754+
const rootRoute = createRootRoute()
4755+
const indexRoute = createRoute({
4756+
getParentRoute: () => rootRoute,
4757+
path: '/',
4758+
component: () => (
4759+
<>
4760+
<h1>Index</h1>
4761+
<CreatedCustomLink to="/posts" target="_self">
4762+
Posts (should navigate internally)
4763+
</CreatedCustomLink>
4764+
</>
4765+
),
4766+
})
4767+
4768+
const postsRoute = createRoute({
4769+
getParentRoute: () => rootRoute,
4770+
path: '/posts',
4771+
component: () => <h1 data-testid="posts-heading">Posts</h1>,
4772+
})
4773+
4774+
const router = createRouter({
4775+
routeTree: rootRoute.addChildren([indexRoute, postsRoute]),
4776+
history: createMemoryHistory({ initialEntries: ['/'] }),
4777+
})
4778+
4779+
render(<RouterProvider router={router} />)
4780+
4781+
const postsLink = await screen.findByRole('link', {
4782+
name: 'Posts (should navigate internally)',
4783+
})
4784+
4785+
expect(postsLink).toHaveAttribute('target', '_self')
4786+
4787+
fireEvent.click(postsLink)
4788+
4789+
await waitFor(() => {
4790+
expect(router.state.location.pathname).toBe('/posts')
4791+
})
4792+
4793+
const postsHeading = await screen.findByTestId('posts-heading')
4794+
expect(postsHeading).toBeInTheDocument()
4795+
})
46864796
})
46874797

46884798
describe('search middleware', () => {

‎packages/solid-router/src/link.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,11 +264,16 @@ export function useLinkProps<
264264

265265
// The click handler
266266
const handleClick = (e: MouseEvent) => {
267+
// Check actual element's target attribute as fallback
268+
const elementTarget = (e.currentTarget as HTMLAnchorElement).target
269+
const effectiveTarget =
270+
local.target !== undefined ? local.target : elementTarget
271+
267272
if (
268273
!local.disabled &&
269274
!isCtrlEvent(e) &&
270275
!e.defaultPrevented &&
271-
(!local.target || local.target === '_self') &&
276+
(!effectiveTarget || effectiveTarget === '_self') &&
272277
e.button === 0
273278
) {
274279
e.preventDefault()

‎packages/solid-router/tests/link.test.tsx‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4245,6 +4245,119 @@ describe('createLink', () => {
42454245
const button3 = await screen.findByText('active: no - foo: no - Button3')
42464246
expect(button3.getAttribute('overrideMeIfYouWant')).toBe('Button3')
42474247
})
4248+
4249+
it('should respect target attribute set by custom component', async () => {
4250+
const CustomLinkWithTarget = (props: {
4251+
href?: string
4252+
children?: Solid.JSX.Element
4253+
ref?: (el: HTMLAnchorElement) => void
4254+
}) => (
4255+
<a ref={props.ref} {...props} target="_blank" rel="noopener noreferrer" />
4256+
)
4257+
4258+
const CreatedCustomLink = createLink(CustomLinkWithTarget)
4259+
4260+
const rootRoute = createRootRoute()
4261+
const indexRoute = createRoute({
4262+
getParentRoute: () => rootRoute,
4263+
path: '/',
4264+
component: () => (
4265+
<>
4266+
<h1>Index</h1>
4267+
<CreatedCustomLink to="/posts">
4268+
Posts (should open in new tab)
4269+
</CreatedCustomLink>
4270+
</>
4271+
),
4272+
})
4273+
4274+
const postsRoute = createRoute({
4275+
getParentRoute: () => rootRoute,
4276+
path: '/posts',
4277+
component: () => <h1 data-testid="posts-heading">Posts</h1>,
4278+
})
4279+
4280+
const router = createRouter({
4281+
routeTree: rootRoute.addChildren([indexRoute, postsRoute]),
4282+
history: createMemoryHistory({ initialEntries: ['/'] }),
4283+
})
4284+
4285+
const originalOpen = window.open
4286+
const openMock = vi.fn()
4287+
window.open = openMock
4288+
4289+
render(() => <RouterProvider router={router} />)
4290+
4291+
const postsLink = await screen.findByRole('link', {
4292+
name: 'Posts (should open in new tab)',
4293+
})
4294+
4295+
expect(postsLink).toHaveAttribute('target', '_blank')
4296+
expect(postsLink).toHaveAttribute('rel', 'noopener noreferrer')
4297+
4298+
fireEvent.click(postsLink)
4299+
4300+
await waitFor(() => {
4301+
expect(router.state.location.pathname).toBe('/')
4302+
})
4303+
4304+
await expect(screen.findByTestId('posts-heading')).rejects.toThrow()
4305+
4306+
window.open = originalOpen
4307+
})
4308+
4309+
it('should allow override of target prop even when custom component sets it', async () => {
4310+
const CustomLinkWithDefaultTarget = (props: {
4311+
href?: string
4312+
children?: Solid.JSX.Element
4313+
target?: string
4314+
ref?: (el: HTMLAnchorElement) => void
4315+
}) => <a ref={props.ref} target="_blank" {...props} />
4316+
4317+
const CreatedCustomLink = createLink(CustomLinkWithDefaultTarget)
4318+
4319+
const rootRoute = createRootRoute()
4320+
const indexRoute = createRoute({
4321+
getParentRoute: () => rootRoute,
4322+
path: '/',
4323+
component: () => (
4324+
<>
4325+
<h1>Index</h1>
4326+
<CreatedCustomLink to="/posts" target="_self">
4327+
Posts (should navigate internally)
4328+
</CreatedCustomLink>
4329+
</>
4330+
),
4331+
})
4332+
4333+
const postsRoute = createRoute({
4334+
getParentRoute: () => rootRoute,
4335+
path: '/posts',
4336+
component: () => <h1 data-testid="posts-heading">Posts</h1>,
4337+
})
4338+
4339+
const router = createRouter({
4340+
routeTree: rootRoute.addChildren([indexRoute, postsRoute]),
4341+
history: createMemoryHistory({ initialEntries: ['/'] }),
4342+
})
4343+
4344+
render(() => <RouterProvider router={router} />)
4345+
4346+
const postsLink = await screen.findByRole('link', {
4347+
name: 'Posts (should navigate internally)',
4348+
})
4349+
4350+
expect(postsLink).toHaveAttribute('target', '_self')
4351+
4352+
fireEvent.click(postsLink)
4353+
4354+
await waitFor(() => {
4355+
expect(router.state.location.pathname).toBe('/posts')
4356+
})
4357+
4358+
const postsHeading = await screen.findByTestId('posts-heading')
4359+
expect(postsHeading).toBeInTheDocument()
4360+
})
42484361
})
42494362

42504363
describe('search middleware', () => {

0 commit comments

Comments
 (0)