Skip to content

Commit 2cd8598

Browse files
SidnioulzSidnioulz
authored
Merge pull request #35311 from yatishgoel/fix/35213-nextjs-link-mock-onclick-order
Next: Call link onClick before preventing default
2 parents 98de385 + 2335034 commit 2cd8598

8 files changed

Lines changed: 135 additions & 10 deletions

File tree

‎code/core/src/actions/loaders.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const logActionsWhenMockCalled: LoaderFunction = (context) => {
3737
'next/router::useRouter()',
3838
'next/navigation::useRouter()',
3939
'next/navigation::redirect',
40+
'next/link::Link',
4041
'next/cache::',
4142
'next/headers::cookies().set',
4243
'next/headers::cookies().delete',

‎code/e2e-sandbox/framework-nextjs.spec.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,28 @@ test.describe('Next.js', () => {
7878
testRoutingBehaviour('Replace', 'replace');
7979
});
8080

81+
test.describe('next/link', () => {
82+
let root: Locator;
83+
let sbPage: SbPage;
84+
85+
test.beforeEach(async ({ page }) => {
86+
sbPage = new SbPage(page, expect);
87+
88+
await sbPage.navigateToStory('stories/frameworks/nextjs/Link', 'triggers-link-action');
89+
root = sbPage.previewRoot();
90+
});
91+
92+
test('should log a link action when clicked', async ({ page }) => {
93+
await root.locator('a', { hasText: 'Trigger link action' }).click();
94+
95+
await sbPage.viewAddonPanel('Actions');
96+
const logItem = page.locator('#storybook-panel-root [role="tabpanel"]', {
97+
hasText: 'next/link::Link',
98+
});
99+
await expect(logItem).toBeVisible();
100+
});
101+
});
102+
81103
test.describe('next/router', () => {
82104
let root: Locator;
83105
let sbPage: SbPage;

‎code/frameworks/nextjs-vite/src/export-mocks/link/index.test.tsx‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { describe, expect, it, vi } from 'vitest';
22

33
import React from 'react';
44
import { renderToStaticMarkup } from 'react-dom/server';
@@ -10,6 +10,21 @@ const renderHref = (props: Record<string, unknown>) => {
1010
return /href="([^"]*)"/.exec(html)?.[1];
1111
};
1212

13+
const makeClickEvent = () => {
14+
const e = {
15+
defaultPrevented: false,
16+
preventDefault() {
17+
e.defaultPrevented = true;
18+
},
19+
};
20+
return e as unknown as React.MouseEvent<HTMLAnchorElement>;
21+
};
22+
23+
const getAnchorOnClick = (props: Record<string, unknown>) =>
24+
(MockLink as any).render(props, null).props.onClick as (
25+
e: React.MouseEvent<HTMLAnchorElement>
26+
) => void;
27+
1328
describe('next/link mock', () => {
1429
it('renders the resolved `as` URL instead of the `href` template when `as` is provided', () => {
1530
expect(renderHref({ href: '/users/[id]', as: '/users/123' })).toBe('/users/123');
@@ -27,4 +42,30 @@ describe('next/link mock', () => {
2742
'/about?ref=nav'
2843
);
2944
});
45+
46+
it('calls onClick before preventing default', () => {
47+
let defaultPreventedWhenHandlerRan: boolean | undefined;
48+
const onClick = vi.fn((e: React.MouseEvent<HTMLAnchorElement>) => {
49+
defaultPreventedWhenHandlerRan = e.defaultPrevented;
50+
});
51+
52+
getAnchorOnClick({ href: '/about', onClick })(makeClickEvent());
53+
54+
expect(onClick).toHaveBeenCalledTimes(1);
55+
expect(defaultPreventedWhenHandlerRan).toBe(false);
56+
});
57+
58+
it('still runs a handler that bails when the event was already default-prevented', () => {
59+
const userOnClick = vi.fn();
60+
const mergedOnClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
61+
if (e.defaultPrevented) {
62+
return;
63+
}
64+
userOnClick();
65+
};
66+
67+
getAnchorOnClick({ href: '/about', onClick: mergedOnClick })(makeClickEvent());
68+
69+
expect(userOnClick).toHaveBeenCalledTimes(1);
70+
});
3071
});

‎code/frameworks/nextjs-vite/src/export-mocks/link/index.tsx‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,28 @@ const MockLink = React.forwardRef<HTMLAnchorElement, any>(function MockLink(
2727
? `${resolvedHref.pathname || ''}${resolvedHref.query ? '?' + new URLSearchParams(resolvedHref.query).toString() : ''}${resolvedHref.hash || ''}`
2828
: resolvedHref;
2929

30-
const handleClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
30+
const navigate = (e: React.MouseEvent<HTMLAnchorElement>) => {
31+
if (e.defaultPrevented) {
32+
return;
33+
}
3134
e.preventDefault();
32-
onClick?.(e);
3335
linkAction(hrefString, { replace, scroll, shallow, prefetch, locale });
3436
};
3537

38+
const handleClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
39+
onClick?.(e);
40+
navigate(e);
41+
};
42+
3643
if (legacyBehavior) {
3744
const child = React.Children.only(children) as React.ReactElement<any>;
3845
const childProps: Record<string, any> = {
3946
ref,
4047
onClick: (e: React.MouseEvent<HTMLAnchorElement>) => {
41-
e.preventDefault();
4248
if (child.props && typeof child.props.onClick === 'function') {
4349
child.props.onClick(e);
4450
}
45-
linkAction(hrefString, { replace, scroll, shallow, prefetch, locale });
51+
navigate(e);
4652
},
4753
...rest,
4854
};

‎code/frameworks/nextjs-vite/template/stories/Link.stories.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ type Story = StoryObj<typeof Component>;
7272

7373
export const Default: Story = {};
7474

75+
export const TriggersLinkAction: Story = {
76+
render: () => <Link href="/dashboard">Trigger link action</Link>,
77+
};
78+
7579
export const InAppDir: Story = {
7680
parameters: {
7781
nextjs: {

‎code/frameworks/nextjs/src/export-mocks/link/index.test.tsx‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { describe, expect, it, vi } from 'vitest';
22

33
import React from 'react';
44
import { renderToStaticMarkup } from 'react-dom/server';
@@ -10,6 +10,21 @@ const renderHref = (props: Record<string, unknown>) => {
1010
return /href="([^"]*)"/.exec(html)?.[1];
1111
};
1212

13+
const makeClickEvent = () => {
14+
const e = {
15+
defaultPrevented: false,
16+
preventDefault() {
17+
e.defaultPrevented = true;
18+
},
19+
};
20+
return e as unknown as React.MouseEvent<HTMLAnchorElement>;
21+
};
22+
23+
const getAnchorOnClick = (props: Record<string, unknown>) =>
24+
(MockLink as any).render(props, null).props.onClick as (
25+
e: React.MouseEvent<HTMLAnchorElement>
26+
) => void;
27+
1328
describe('next/link mock', () => {
1429
it('renders the resolved `as` URL instead of the `href` template when `as` is provided', () => {
1530
expect(renderHref({ href: '/users/[id]', as: '/users/123' })).toBe('/users/123');
@@ -27,4 +42,30 @@ describe('next/link mock', () => {
2742
'/about?ref=nav'
2843
);
2944
});
45+
46+
it('calls onClick before preventing default', () => {
47+
let defaultPreventedWhenHandlerRan: boolean | undefined;
48+
const onClick = vi.fn((e: React.MouseEvent<HTMLAnchorElement>) => {
49+
defaultPreventedWhenHandlerRan = e.defaultPrevented;
50+
});
51+
52+
getAnchorOnClick({ href: '/about', onClick })(makeClickEvent());
53+
54+
expect(onClick).toHaveBeenCalledTimes(1);
55+
expect(defaultPreventedWhenHandlerRan).toBe(false);
56+
});
57+
58+
it('still runs a handler that bails when the event was already default-prevented', () => {
59+
const userOnClick = vi.fn();
60+
const mergedOnClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
61+
if (e.defaultPrevented) {
62+
return;
63+
}
64+
userOnClick();
65+
};
66+
67+
getAnchorOnClick({ href: '/about', onClick: mergedOnClick })(makeClickEvent());
68+
69+
expect(userOnClick).toHaveBeenCalledTimes(1);
70+
});
3071
});

‎code/frameworks/nextjs/src/export-mocks/link/index.tsx‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,28 @@ const MockLink = React.forwardRef<HTMLAnchorElement, any>(function MockLink(
2727
? `${resolvedHref.pathname || ''}${resolvedHref.query ? '?' + new URLSearchParams(resolvedHref.query).toString() : ''}${resolvedHref.hash || ''}`
2828
: resolvedHref;
2929

30-
const handleClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
30+
const navigate = (e: React.MouseEvent<HTMLAnchorElement>) => {
31+
if (e.defaultPrevented) {
32+
return;
33+
}
3134
e.preventDefault();
32-
onClick?.(e);
3335
linkAction(hrefString, { replace, scroll, shallow, prefetch, locale });
3436
};
3537

38+
const handleClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
39+
onClick?.(e);
40+
navigate(e);
41+
};
42+
3643
if (legacyBehavior) {
3744
const child = React.Children.only(children) as React.ReactElement<any>;
3845
const childProps: Record<string, any> = {
3946
ref,
4047
onClick: (e: React.MouseEvent<HTMLAnchorElement>) => {
41-
e.preventDefault();
4248
if (child.props && typeof child.props.onClick === 'function') {
4349
child.props.onClick(e);
4450
}
45-
linkAction(hrefString, { replace, scroll, shallow, prefetch, locale });
51+
navigate(e);
4652
},
4753
...rest,
4854
};

‎code/frameworks/nextjs/template/stories/Link.stories.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ type Story = StoryObj<typeof Component>;
7272

7373
export const Default: Story = {};
7474

75+
export const TriggersLinkAction: Story = {
76+
render: () => <Link href="/dashboard">Trigger link action</Link>,
77+
};
78+
7579
export const InAppDir: Story = {
7680
parameters: {
7781
nextjs: {

0 commit comments

Comments
 (0)