Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NEXT-588] fetch to App Route + router.refresh() causes crash #45956

Closed
1 task done
Josehower opened this issue Feb 15, 2023 · 7 comments · Fixed by #46102
Closed
1 task done

[NEXT-588] fetch to App Route + router.refresh() causes crash #45956

Josehower opened this issue Feb 15, 2023 · 7 comments · Fixed by #46102
Labels
linear: next Confirmed issue that is tracked by the Next.js team.

Comments

@Josehower
Copy link
Contributor

Josehower commented Feb 15, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

➜  api-app-dir git:(main) yarn next info
yarn run v1.22.19
$ /home/josehower/upleveled-projects/api-app-dir/node_modules/.bin/next info

    Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP Wed Nov 23 01:01:46 UTC 2022
    Binaries:
      Node: 16.19.0
      npm: 8.19.3
      Yarn: 1.22.19
      pnpm: 7.18.2
    Relevant packages:
      next: 13.1.7-canary.15
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0

Done in 1.76s.

Which area(s) of Next.js are affected? (leave empty if unsure)

No response

Link to the code that reproduces this issue

https://stackblitz.com/edit/vercel-next-js-mg1bxt?file=app%2Fpage.tsx,app%2Flayout.tsx

To Reproduce

The reproduction example contains a single button that perform a fetch request to a very simple API route in app/apizz/route.ts.

router.refresh() breaks the app if you use it after the fetch.

ready - started server on 0.0.0.0:3000, url: http://localhost:3000
warn  - You have enabled experimental feature (appDir) in next.config.js.
warn  - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use at your own risk.      

info  - Thank you for testing `appDir` please leave your feedback at https://nextjs.link/app-feedback
event - compiled client and server successfully in 446 ms (247 modules)
wait  - compiling /page (client and server)...
event - compiled client and server successfully in 312 ms (438 modules)
wait  - compiling /api/route (client and server)...
event - compiled client and server successfully in 103 ms (403 modules)
error - node_modules/next/dist/compiled/react-server-dom-webpack/server.browser.js (229:60) @ clientReference
error - TypeError: Cannot read properties of undefined (reading '')
    at stringify (<anonymous>)
null
error - node_modules/next/dist/compiled/react-server-dom-webpack/server.browser.js (229:60) @ clientReference
error - TypeError: Cannot read properties of undefined (reading '')
    at stringify (<anonymous>)
null

This doesn't happen if the API route exists in /pages/api

https://stackblitz.com/edit/vercel-next-js-mt4boz?file=app%2Fpage.tsx

Describe the Bug

The bug break the app when we try a redirection using router.push() after a fetch request to an API route in app/ directory

Expected Behavior

api routes in app directory should allow a redirection after a fetch request similar to pages/api.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

NEXT-588

@Josehower Josehower added the bug Issue was opened via the bug report template. label Feb 15, 2023
@zhyd1997
Copy link

the bug occurred here:

var resolvedModuleData = config[clientReference.filepath][clientReference.name];

config always stores files in appDir, which is not consistent with filePath (possible is node_modules/next/dist/client/*).

@karlhorky
Copy link
Contributor

Cc @huozhi

@Josehower Josehower changed the title App Routes not working as expected on router.refresh() fetch to App Route + router.refresh() causes crash Feb 17, 2023
@karlhorky
Copy link
Contributor

karlhorky commented Feb 17, 2023

This still occurs on [email protected], new demo:

StackBlitz demo: https://stackblitz.com/edit/vercel-next-js-vyumbh?file=app%2Fpage.tsx

@karlhorky
Copy link
Contributor

Seems like other users are also experiencing this:

@karlhorky
Copy link
Contributor

This still occurs on [email protected], new demo:

StackBlitz demo: https://stackblitz.com/edit/vercel-next-js-wzdzon?file=app%2Fpage.tsx

@timneutkens timneutkens added linear: next Confirmed issue that is tracked by the Next.js team. bug and removed bug Issue was opened via the bug report template. labels Feb 18, 2023
@timneutkens timneutkens changed the title fetch to App Route + router.refresh() causes crash [NEXT-588] fetch to App Route + router.refresh() causes crash Feb 18, 2023
timneutkens added a commit to timneutkens/next.js that referenced this issue Feb 18, 2023
timneutkens added a commit that referenced this issue Feb 18, 2023
This ensures there is no client component entry created for route.js.
@shuding is going to investigate further why this would break the
manifest generation in development.


Fixes #45956
Fixes NEXT-588

<!--
Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:
-->

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ]
[e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
vorcigernix pushed a commit to vorcigernix/next.js that referenced this issue Feb 19, 2023
This ensures there is no client component entry created for route.js.
@shuding is going to investigate further why this would break the
manifest generation in development.


Fixes vercel#45956
Fixes NEXT-588

<!--
Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:
-->

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ]
[e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
@karlhorky
Copy link
Contributor

Great, just confirmed that #46102 works! (released in [email protected]). New demo:

StackBlitz demo: https://stackblitz.com/edit/vercel-next-js-r58uun?file=app%2Fpage.tsx,app%2Flayout.tsx

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
linear: next Confirmed issue that is tracked by the Next.js team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants