a65ea447d8
### What? Fixes #58570 ### How? Include the **basePath** to the **fetchUrl** to ensure the relative URL matches the app when deployed under a **basePath**. ### Tested? I have added an **e2e** test with a basic custom server & server action redirect. This test was confirmed to **catch** the bug when running it without the fix in place. When running it you will get the failed result. ``` FAIL test/e2e/app-dir/app-basepath-custom-server/index.test.ts (12.293 s) custom-app-render ✕ redirects properly when server action handler uses `redirect` (1661 ms) ● custom-app-render › redirects properly when server action handler uses `redirect` expect(received).not.toEqual(expected) // deep equality Expected: not ["/base/another", 200] 45 | // if broken, this will include a 200 from the /base/another indicating a full page redirect 46 | responses.forEach((res) => { > 47 | expect(res).not.toEqual(['/base/another', 200]) | ^ 48 | }) 49 | }) 50 | } at toEqual (e2e/app-dir/app-basepath-custom-server/index.test.ts:47:25) at Array.forEach (<anonymous>) at Object.forEach (e2e/app-dir/app-basepath-custom-server/index.test.ts:46:17) Test Suites: 1 failed, 1 total Tests: 1 failed, 1 total Snapshots: 0 total Time: 12.321 s, estimated 22 s Ran all test suites matching /test\/e2e\/app-dir\/app-basepath-custom-server/i. ELIFECYCLE Command failed with exit code 1. ELIFECYCLE Command failed with exit code 1. ELIFECYCLE Command failed with exit code 1. ``` ### Notes Not sure if there are any edge cases where the `fetchUrl` is now broken in other use cases where there is no **basePath**, I assume the string would be empty `""` and result in the same URL as before, but not sure? ### Disclosure ~I am not that familiar with the Next.js code base and this is my first PR. I was struggling to find out how to grab the **basePath** from `next.config.js`, but then noticed the **assetPrefix** inside the function matched, so decided to use that for minimal change. I don't know if there are any caveats with this approach, but could consider switching to pull directly from the config file, if that's possible?~ **Update:** Figured out where the **basePath** came from and switched it. --------- Co-authored-by: Colton Ehrman <cehrman@paypal.com> Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
39 lines
1.2 KiB
TypeScript
39 lines
1.2 KiB
TypeScript
import { join } from 'path'
|
|
import { createNextDescribe } from 'e2e-utils'
|
|
import { retry } from 'next-test-utils'
|
|
|
|
createNextDescribe(
|
|
'custom-app-server-action-redirect',
|
|
{
|
|
files: join(__dirname, 'custom-server'),
|
|
skipDeployment: true,
|
|
startCommand: 'node server.js',
|
|
dependencies: {
|
|
'get-port': '5.1.1',
|
|
},
|
|
},
|
|
({ next }) => {
|
|
it('redirects with basepath properly when server action handler uses `redirect`', async () => {
|
|
const browser = await next.browser('/base')
|
|
const getCount = async () => browser.elementByCss('#current-count').text()
|
|
|
|
// Increase count to track if the page reloaded
|
|
await browser.elementByCss('#increase-count').click().click()
|
|
await retry(async () => {
|
|
expect(await getCount()).toBe('Count: 2')
|
|
})
|
|
|
|
await browser.elementById('submit-server-action-redirect').click()
|
|
|
|
expect(await browser.waitForElementByCss('#another').text()).toBe(
|
|
'Another Page'
|
|
)
|
|
expect(await browser.url()).toBe(
|
|
`http://localhost:${next.appPort}/base/another`
|
|
)
|
|
|
|
// Count should still be 2 as the browser should not have reloaded the page.
|
|
expect(await getCount()).toBe('Count: 2')
|
|
})
|
|
}
|
|
)
|