fix: redirect logic missing basePath in App Render #60184

coltonehrman · 2024-01-03T20:43:32Z

What?

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.

timneutkens

Hey Colton, these changes look good to me 💯

The only thing missing is a test to ensure it doesn't regress in the future. You can find how to add and run tests here: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md

We have a large number of Server Actions tests in these directories:

And the App Router tests are here:

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir

You'll probably want to add this particular change in https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/app-basepath which is already testing other basePath related features 👍

If that doesn't work out I can help 🙌

ijjk · 2024-01-10T15:01:27Z

Allow CI Workflow Run

approve CI run for commit: b7919b6

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

Include the basePath to the fetchUrl to ensure the relative URL matches the app when deployed under a basePath. - Fixes #58570

coltonehrman · 2024-01-10T17:51:10Z

Hey Colton, these changes look good to me 💯

The only thing missing is a test to ensure it doesn't regress in the future. You can find how to add and run tests here: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md

We have a large number of Server Actions tests in these directories:

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/actions

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/actions-navigation

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/actions-allowed-origins

And the App Router tests are here:

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir

You'll probably want to add this particular change in https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/app-basepath which is already testing other basePath related features 👍

If that doesn't work out I can help 🙌

Hey @timneutkens, thank you for the feedback and guidance!

I am attempting to run tests now locally and seem to be having some struggles. I have already rebased my branch to latest upsteam/canary, re-installed using pnpm i, built using pnpm build and tried running both pnpm test-start and pnpm testonly-start.

I get this error tree-kill Error: pid must be a number along with a timeout error on every test where they all fail.
thrown: "Exceeded timeout of 120000 ms for a hook.

Full Log w/ Error

pnpm test-start test/e2e/app-dir/actions/app-action.test.ts 

> [email protected] test-start /Users/cehrman/Desktop/WebPlatform/next.js
> cross-env NEXT_TEST_MODE=start pnpm testheadless "test/e2e/app-dir/actions/app-action.test.ts"


> [email protected] testheadless /Users/cehrman/Desktop/WebPlatform/next.js
> cross-env HEADLESS=true pnpm testonly "test/e2e/app-dir/actions/app-action.test.ts"


> [email protected] testonly /Users/cehrman/Desktop/WebPlatform/next.js
> jest --runInBand "test/e2e/app-dir/actions/app-action.test.ts"

Using test mode: start in test folder e2e
Creating test directory with isolated next... (use NEXT_SKIP_ISOLATE=1 to opt-out)
Creating next instance in:
/private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-install-6e4b3763c3a9f3af58ead1b950bf28039f2b942f4cd6e0690d208addeaf954fd
Creating temp repo dir /private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-repo-c651b0f94822d741166d85d0e679bda7cefb4718669caf3c48762bd181bbb2f2
  console.log
    using swc dep {
      nextSwcVersion: null,
      nextSwcPkg: {
        packageJsonPath: '/private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-repo-c651b0f94822d741166d85d0e679bda7cefb4718669caf3c48762bd181bbb2f2/packages/next-swc/package.json',
        packagePath: '/private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-repo-c651b0f94822d741166d85d0e679bda7cefb4718669caf3c48762bd181bbb2f2/packages/next-swc',
        packageJson: {
          name: '@next/swc',
          version: '14.0.5-canary.47',
          private: true,
          scripts: [Object],
          napi: [Object],
          devDependencies: [Object]
        },
        packedPackageTarPath: '/private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-repo-c651b0f94822d741166d85d0e679bda7cefb4718669caf3c48762bd181bbb2f2/packages/next-swc/next-swc-packed.tgz'
      }
    }

      at log (../.github/actions/next-stats-action/src/prepare/repo-setup.js:147:19)

using swc binaries:  next-swc.darwin-arm64.node, next-swc.darwin-x64.node, next-swc.linux-arm64-gnu.node, next-swc.linux-arm64-musl.node, next-swc.linux-x64-gnu.node, next-swc.linux-x64-musl.node, next-swc.win32-arm64-msvc.node, next-swc.win32-ia32-msvc.node, next-swc.win32-x64-msvc.node

 RUNS  test/e2e/app-dir/actions/app-action.test.ts
Packages: +30
++++++++++++++++++++++++++++++
Progress: resolved 30, reused 27, downloaded 3, added 30, done

dependencies:
+ @types/node 20.10.8
+ @types/react 18.2.47
+ @types/react-dom 18.2.18
+ nanoid 5.0.4
+ next 14.0.5-canary.47
+ react 18.2.0
+ react-dom 18.2.0
+ server-only 0.0.1
+ typescript 5.3.3
created next.js install, writing test files
  console.log
    running yarn next build

      at NextStartInstance.log [as start] (lib/next-modes/next-start.ts:78:13)

tree-kill Error: pid must be a number
    at Object.<anonymous>.module.exports (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/tree-kill/index.js:16:29)
    at /Users/cehrman/Desktop/WebPlatform/next.js/test/lib/next-modes/base.ts:362:17
    at new Promise (<anonymous>)
    at NextStartInstance.stop (/Users/cehrman/Desktop/WebPlatform/next.js/test/lib/next-modes/base.ts:361:13)
    at NextStartInstance.stop [as destroy] (/Users/cehrman/Desktop/WebPlatform/next.js/test/lib/next-modes/base.ts:383:18)
    at Object.destroy (/Users/cehrman/Desktop/WebPlatform/next.js/test/lib/e2e-utils.ts:119:26)
    at Promise.then.completed (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/utils.js:231:10)
    at _callCircusHook (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/run.js:281:40)
    at _runTestsForDescribeBlock (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/run.js:154:7)
    at run (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-runner/build/runTest.js:367:16)
    at runTest (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-runner/build/runTest.js:444:34)
 FAIL  test/e2e/app-dir/actions/app-action.test.ts (240.671 s)
  ● app-dir action handling › should handle basic actions correctly

    thrown: "Exceeded timeout of 120000 ms for a hook.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

coltonehrman · 2024-01-10T19:08:58Z

Hey Colton, these changes look good to me 💯
The only thing missing is a test to ensure it doesn't regress in the future. You can find how to add and run tests here: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md
We have a large number of Server Actions tests in these directories:

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/actions

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/actions-navigation

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/actions-allowed-origins

And the App Router tests are here:

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir

You'll probably want to add this particular change in https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/app-basepath which is already testing other basePath related features 👍
If that doesn't work out I can help 🙌

Hey @timneutkens, thank you for the feedback and guidance!

I am attempting to run tests now locally and seem to be having some struggles. I have already rebased my branch to latest upsteam/canary, re-installed using pnpm i, built using pnpm build and tried running both pnpm test-start and pnpm testonly-start.

I get this error tree-kill Error: pid must be a number along with a timeout error on every test where they all fail. thrown: "Exceeded timeout of 120000 ms for a hook.

Full Log w/ Error

pnpm test-start test/e2e/app-dir/actions/app-action.test.ts 

> [email protected] test-start /Users/cehrman/Desktop/WebPlatform/next.js
> cross-env NEXT_TEST_MODE=start pnpm testheadless "test/e2e/app-dir/actions/app-action.test.ts"


> [email protected] testheadless /Users/cehrman/Desktop/WebPlatform/next.js
> cross-env HEADLESS=true pnpm testonly "test/e2e/app-dir/actions/app-action.test.ts"


> [email protected] testonly /Users/cehrman/Desktop/WebPlatform/next.js
> jest --runInBand "test/e2e/app-dir/actions/app-action.test.ts"

Using test mode: start in test folder e2e
Creating test directory with isolated next... (use NEXT_SKIP_ISOLATE=1 to opt-out)
Creating next instance in:
/private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-install-6e4b3763c3a9f3af58ead1b950bf28039f2b942f4cd6e0690d208addeaf954fd
Creating temp repo dir /private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-repo-c651b0f94822d741166d85d0e679bda7cefb4718669caf3c48762bd181bbb2f2
  console.log
    using swc dep {
      nextSwcVersion: null,
      nextSwcPkg: {
        packageJsonPath: '/private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-repo-c651b0f94822d741166d85d0e679bda7cefb4718669caf3c48762bd181bbb2f2/packages/next-swc/package.json',
        packagePath: '/private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-repo-c651b0f94822d741166d85d0e679bda7cefb4718669caf3c48762bd181bbb2f2/packages/next-swc',
        packageJson: {
          name: '@next/swc',
          version: '14.0.5-canary.47',
          private: true,
          scripts: [Object],
          napi: [Object],
          devDependencies: [Object]
        },
        packedPackageTarPath: '/private/var/folders/1r/tqbwmgt125ncrfr2009b12vh0000gp/T/next-repo-c651b0f94822d741166d85d0e679bda7cefb4718669caf3c48762bd181bbb2f2/packages/next-swc/next-swc-packed.tgz'
      }
    }

      at log (../.github/actions/next-stats-action/src/prepare/repo-setup.js:147:19)

using swc binaries:  next-swc.darwin-arm64.node, next-swc.darwin-x64.node, next-swc.linux-arm64-gnu.node, next-swc.linux-arm64-musl.node, next-swc.linux-x64-gnu.node, next-swc.linux-x64-musl.node, next-swc.win32-arm64-msvc.node, next-swc.win32-ia32-msvc.node, next-swc.win32-x64-msvc.node

 RUNS  test/e2e/app-dir/actions/app-action.test.ts
Packages: +30
++++++++++++++++++++++++++++++
Progress: resolved 30, reused 27, downloaded 3, added 30, done

dependencies:
+ @types/node 20.10.8
+ @types/react 18.2.47
+ @types/react-dom 18.2.18
+ nanoid 5.0.4
+ next 14.0.5-canary.47
+ react 18.2.0
+ react-dom 18.2.0
+ server-only 0.0.1
+ typescript 5.3.3
created next.js install, writing test files
  console.log
    running yarn next build

      at NextStartInstance.log [as start] (lib/next-modes/next-start.ts:78:13)

tree-kill Error: pid must be a number
    at Object.<anonymous>.module.exports (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/tree-kill/index.js:16:29)
    at /Users/cehrman/Desktop/WebPlatform/next.js/test/lib/next-modes/base.ts:362:17
    at new Promise (<anonymous>)
    at NextStartInstance.stop (/Users/cehrman/Desktop/WebPlatform/next.js/test/lib/next-modes/base.ts:361:13)
    at NextStartInstance.stop [as destroy] (/Users/cehrman/Desktop/WebPlatform/next.js/test/lib/next-modes/base.ts:383:18)
    at Object.destroy (/Users/cehrman/Desktop/WebPlatform/next.js/test/lib/e2e-utils.ts:119:26)
    at Promise.then.completed (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/utils.js:231:10)
    at _callCircusHook (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/run.js:281:40)
    at _runTestsForDescribeBlock (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/run.js:154:7)
    at run (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-runner/build/runTest.js:367:16)
    at runTest (/Users/cehrman/Desktop/WebPlatform/next.js/node_modules/.pnpm/[email protected]/node_modules/jest-runner/build/runTest.js:444:34)
 FAIL  test/e2e/app-dir/actions/app-action.test.ts (240.671 s)
  ● app-dir action handling › should handle basic actions correctly

    thrown: "Exceeded timeout of 120000 ms for a hook.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

UPDATE: fixed by using NEXT_TEST_SKIP_CLEANUP=1 NEXT_SKIP_ISOLATE=1

coltonehrman · 2024-01-10T19:56:24Z

@timneutkens Do you have any good examples of tests that include a custom server? This bug appears to only be reproducible with a custom server since the logic inside the redirect does not care about what path is chosen, only if the response is a RSC text/x-component or not, and with Next.js, you still get back a text/x-component response when hitting the root / of an app with a basePath set (ie: not the base root, but the root root).

I need to be able to interact with the page after render, but can't seem to find a custom server test that does this anywhere, only does a render and checks the raw response.

it('redirects properly when server action handler uses `redirect`', async () => {
    const postRequests = []
    const responseCodes = []

    const browser = await next.browser('/base/actions-redirect', {
      beforePageLoad(page) {
        page.on('request', (request: Request) => {
          const url = new URL(request.url())
          if (request.method() === 'POST') {
            postRequests.push(`${url.pathname}${url.search}`)
          }
        })

        page.on('response', (response: Response) => {
          const url = new URL(response.url())
          const status = response.status()

          console.log(url, status)
          console.log(response.headers())

          if (postRequests.includes(`${url.pathname}${url.search}`)) {
            console.log(url)
            responseCodes.push(status)
          }
        })
      },
    })

    await browser.elementById('submit-server-action-redirect').click()
    await check(() => browser.url(), /another/)

    expect(postRequests).toEqual([
      `/base/actions-redirect`,
    ])

    // expect(responseCodes).toEqual([200])
  })

coltonehrman · 2024-01-10T20:55:52Z

Hey Colton, these changes look good to me 💯

The only thing missing is a test to ensure it doesn't regress in the future. You can find how to add and run tests here: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md

We have a large number of Server Actions tests in these directories:

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/actions

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/actions-navigation

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/actions-allowed-origins

And the App Router tests are here:

https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir

You'll probably want to add this particular change in https://github.com/vercel/next.js/tree/canary/test/e2e/app-dir/app-basepath which is already testing other basePath related features 👍

If that doesn't work out I can help 🙌

@timneutkens Sorry for all the pings...

I have added an e2e test that I believe should fit the criteria here, let me know what you think :D

timneutkens

The test looks great! Awesome work 💯

I made some slight tweaks to ensure it doesn't rely on internal implementation details and made sure that we check the entire URL when navigated in case the basePath would ever be wrong.

ijjk · 2024-01-11T10:03:09Z

Stats from current PR

Default Build

General

	vercel/next.js canary	coltonehrman/next.js fix/redirect-basepath	Change
buildDuration	12.7s	12.9s	⚠️ +226ms
buildDurationCached	7s	5.9s	N/A
nodeModulesSize	200 MB	199 MB	N/A
nextStartRea..uration (ms)	426ms	427ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	coltonehrman/next.js fix/redirect-basepath	Change
193.HASH.js gzip	181 B	182 B	N/A
3f784ff6-HASH.js gzip	53.3 kB	53.3 kB	N/A
433-HASH.js gzip	28.5 kB	28.6 kB	N/A
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	240 B	242 B	N/A
main-HASH.js gzip	31.8 kB	31.8 kB	N/A
webpack-HASH.js gzip	1.7 kB	1.7 kB	N/A
Overall change	45.2 kB	45.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	coltonehrman/next.js fix/redirect-basepath	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	coltonehrman/next.js fix/redirect-basepath	Change
_app-HASH.js gzip	194 B	195 B	N/A
_error-HASH.js gzip	183 B	181 B	N/A
amp-HASH.js gzip	504 B	502 B	N/A
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.5 kB	2.5 kB	N/A
edge-ssr-HASH.js gzip	255 B	253 B	N/A
head-HASH.js gzip	350 B	349 B	N/A
hooks-HASH.js gzip	369 B	369 B	✓
image-HASH.js gzip	4.28 kB	4.28 kB	N/A
index-HASH.js gzip	255 B	256 B	N/A
link-HASH.js gzip	2.61 kB	2.61 kB	✓
routerDirect..HASH.js gzip	312 B	311 B	N/A
script-HASH.js gzip	385 B	383 B	N/A
withRouter-HASH.js gzip	307 B	308 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	3.4 kB	3.4 kB	✓

Client Build Manifests

	vercel/next.js canary	coltonehrman/next.js fix/redirect-basepath	Change
_buildManifest.js gzip	483 B	484 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	coltonehrman/next.js fix/redirect-basepath	Change
index.html gzip	528 B	528 B	✓
link.html gzip	540 B	540 B	✓
withRouter.html gzip	523 B	523 B	✓
Overall change	1.59 kB	1.59 kB	✓

Edge SSR bundle Size

	vercel/next.js canary	coltonehrman/next.js fix/redirect-basepath	Change
edge-ssr.js gzip	93.9 kB	93.8 kB	N/A
page.js gzip	148 kB	148 kB	✓
Overall change	148 kB	148 kB	✓

Middleware size

	vercel/next.js canary	coltonehrman/next.js fix/redirect-basepath	Change
middleware-b..fest.js gzip	623 B	624 B	N/A
middleware-r..fest.js gzip	151 B	151 B	✓
middleware.js gzip	37.5 kB	37.5 kB	N/A
edge-runtime..pack.js gzip	1.92 kB	1.92 kB	✓
Overall change	2.07 kB	2.07 kB	✓

Next Runtimes

	vercel/next.js canary	coltonehrman/next.js fix/redirect-basepath	Change
app-page-exp...dev.js gzip	169 kB	169 kB	N/A
app-page-exp..prod.js gzip	95.3 kB	95.3 kB	N/A
app-page-tur..prod.js gzip	96 kB	96 kB	N/A
app-page-tur..prod.js gzip	90.6 kB	90.6 kB	N/A
app-page.run...dev.js gzip	142 kB	142 kB	N/A
app-page.run..prod.js gzip	89.9 kB	89.9 kB	N/A
app-route-ex...dev.js gzip	24.1 kB	24.1 kB	✓
app-route-ex..prod.js gzip	16.7 kB	16.7 kB	✓
app-route-tu..prod.js gzip	16.7 kB	16.7 kB	✓
app-route-tu..prod.js gzip	16.3 kB	16.3 kB	✓
app-route.ru...dev.js gzip	23.5 kB	23.5 kB	✓
app-route.ru..prod.js gzip	16.3 kB	16.3 kB	✓
pages-api-tu..prod.js gzip	9.38 kB	9.38 kB	✓
pages-api.ru...dev.js gzip	9.65 kB	9.65 kB	✓
pages-api.ru..prod.js gzip	9.37 kB	9.37 kB	✓
pages-turbo...prod.js gzip	21.9 kB	21.9 kB	✓
pages.runtim...dev.js gzip	22.5 kB	22.5 kB	✓
pages.runtim..prod.js gzip	21.9 kB	21.9 kB	✓
server.runti..prod.js gzip	49.5 kB	49.5 kB	✓
Overall change	258 kB	258 kB	✓

Diff details

Diff for page.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

Diff too large to display

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

Diff too large to display

Diff for app-page.runtime.prod.js

Diff too large to display

Commit: 58a6a4c

coltonehrman requested review from timneutkens, ijjk, shuding, huozhi, feedthejim, ztanner and wyattjoh as code owners January 3, 2024 20:43

ijjk added the type: next label Jan 3, 2024

coltonehrman changed the title ~~fix: redirect logic for App Render~~ fix: redirect logic missing assetPrefix/basePath in App Render Jan 3, 2024

coltonehrman changed the title ~~fix: redirect logic missing assetPrefix/basePath in App Render~~ fix: redirect logic missing basePath in App Render Jan 3, 2024

timneutkens requested changes Jan 10, 2024

View reviewed changes

fix: redirect logic for App Render

9f7fa0b

Include the basePath to the fetchUrl to ensure the relative URL matches the app when deployed under a basePath. - Fixes #58570

test: include app basepath custom server example w/ test

1ff0d4d

ijjk added the area: tests label Jan 10, 2024

coltonehrman requested a review from timneutkens January 10, 2024 20:56

timneutkens added 2 commits January 11, 2024 10:39

Use waitForElementByCss instead of check()

af59027

Update test to check if the page reloaded differently

eca4f6b

timneutkens added the CI approved Approve running CI for fork label Jan 11, 2024

timneutkens approved these changes Jan 11, 2024

View reviewed changes

Check page text too

58a6a4c

feedthejim enabled auto-merge (squash) January 11, 2024 10:06

feedthejim merged commit a65ea44 into vercel:canary Jan 11, 2024
61 of 66 checks passed

vordgi mentioned this pull request Jan 21, 2024

Remove obsolete ("remove after next minor") #60930

Open

github-actions bot added the locked label Jan 26, 2024

github-actions bot locked as resolved and limited conversation to collaborators Jan 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: redirect logic missing basePath in App Render #60184

fix: redirect logic missing basePath in App Render #60184

coltonehrman commented Jan 3, 2024 •

edited

Loading

timneutkens left a comment

ijjk commented Jan 10, 2024

coltonehrman commented Jan 10, 2024 •

edited

Loading

coltonehrman commented Jan 10, 2024

Full Log w/ Error

coltonehrman commented Jan 10, 2024 •

edited

Loading

coltonehrman commented Jan 10, 2024

timneutkens left a comment

ijjk commented Jan 11, 2024

fix: redirect logic missing basePath in App Render #60184

fix: redirect logic missing basePath in App Render #60184

Conversation

coltonehrman commented Jan 3, 2024 • edited Loading

What?

How?

Tested?

Notes

Disclosure

timneutkens left a comment

Choose a reason for hiding this comment

ijjk commented Jan 10, 2024

coltonehrman commented Jan 10, 2024 • edited Loading

Full Log w/ Error

coltonehrman commented Jan 10, 2024

Full Log w/ Error

coltonehrman commented Jan 10, 2024 • edited Loading

coltonehrman commented Jan 10, 2024

timneutkens left a comment

Choose a reason for hiding this comment

ijjk commented Jan 11, 2024

Stats from current PR

coltonehrman commented Jan 3, 2024 •

edited

Loading

coltonehrman commented Jan 10, 2024 •

edited

Loading

coltonehrman commented Jan 10, 2024 •

edited

Loading