Commit graph

1 commit

Author SHA1 Message Date
Colton Ehrman
a65ea447d8
fix: redirect logic missing basePath in App Render (#60184)
### 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>
2024-01-11 10:08:29 +00:00