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-963] fix not-found on navigation (prod/dev) #47862

Closed
1 task done
TheEdoRan opened this issue Apr 3, 2023 · 15 comments · Fixed by #49855
Closed
1 task done

[NEXT-963] fix not-found on navigation (prod/dev) #47862

TheEdoRan opened this issue Apr 3, 2023 · 15 comments · Fixed by #49855
Assignees
Labels
area: app App directory (appDir: true) linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@TheEdoRan
Copy link

TheEdoRan commented Apr 3, 2023

Verify canary release

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

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP Fri Jan 27 02:56:13 UTC 2023
    Binaries:
      Node: 18.14.2
      npm: 9.6.3
      Yarn: 1.22.19
      pnpm: 8.1.0
    Relevant packages:
      next: 13.2.5-canary.28
      eslint-config-next: 13.2.4
      react: 18.2.0
      react-dom: 18.2.0

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

App directory (appDir: true)

Link to the code that reproduces this issue

https://github.com/TheEdoRan/nextjs-appdir-notfound-bug

To Reproduce

Import a CSS file in the root layout.tsx. Then create a not-found.tsx page inside app/ dir, and navigate to a non-existent page.

Describe the Bug

When navigating to a non-existent page, the HTML content of not-found.tsx shows up correctly, but CSS doesn't load.
Furthermore, clicking on a <Link /> updates the current URL in the browser, but the page does not refresh, so the not-found body is still displayed.

If the CSS file is also imported in not-found.tsx, when navigating to a non-existent page the browser "flashes" the correct not-found page, but then it immediately switches to a blank render.

After a bit of research, I found out that the bug first appeared in v13.2.5-canary.22, in canary.21 the not-found page works as intended.

Expected Behavior

not-found page applies styles and refreshes the page when clicking on a <Link />.

Which browser are you using? (if relevant)

Chrome 111.0.5563.147

How are you deploying your application? (if relevant)

No response

NEXT-963

@TheEdoRan TheEdoRan added the bug Issue was opened via the bug report template. label Apr 3, 2023
@github-actions github-actions bot added the area: app App directory (appDir: true) label Apr 3, 2023
shuding pushed a commit that referenced this issue Apr 6, 2023
### What

This issue is introduced in #47688, we need to do the same work for
rendering single component which collecting the assets and then render
with root layout + root not found

Fix #47970
Related #47862 (partially fix the css issue but not link issue)

### How

This PR encapsulates the preload and stylesheets assets collection and
rendering process, and move them into a helper, and share between the
component rendering and the root not found rendering
@huozhi huozhi added kind: bug linear: next Confirmed issue that is tracked by the Next.js team. and removed bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. labels Apr 6, 2023
@huozhi
Copy link
Member

huozhi commented Apr 6, 2023

I've partically fixed this issue in #47992, only the CSS part of it. Will edit the title to narrow down on the Link issue and keep tracking

@huozhi huozhi changed the title not-found page doesn't load CSS and Link doesn't work in it Link doesn't work in it in root not-found Apr 6, 2023
@jankaifer jankaifer removed kind: bug linear: next Confirmed issue that is tracked by the Next.js team. labels Apr 7, 2023
@huozhi huozhi added the linear: next Confirmed issue that is tracked by the Next.js team. label Apr 7, 2023
@timneutkens timneutkens changed the title Link doesn't work in it in root not-found [NEXT-963] Link doesn't work in it in root not-found Apr 7, 2023
@sanskar-19
Copy link

@timneutkens Can you please update us on the issue and when its patch is gonna come?

timneutkens added a commit that referenced this issue May 18, 2023
## What?

Currently the error boundary state does not reset when a navigation
happens because the `key` does not change. Because of that navigating
using next/link when the error boundary is in the open state ends up not
rendering the contents of the new navigation, instead it keeps the error
boundary contents.

## How?

This PR uses a pattern that is recommended in the React docs (setState
during render) to reset the error boundary if the `usePathname` value
has changed, it is used as a proxy for a new navigation.

Fixes #47862
Fixes NEXT-963
Fixes #49736
Fixes #49732
Fixes #49599


<!-- 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:

## For Contributors

### Improving Documentation or adding/fixing Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

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



## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->
@Afrozefathima
Copy link

Afrozefathima commented Jun 5, 2023

For me none of the Link works in production. Working only in development after upgrading from next js 12 to 13.4.2

@grzegorzpokorski
Copy link

For me none of the Link works in production. Working only in development after upgrading from next js 12 to 13.4.2

@Afrozefathima If I'm not wrong, this issue has been resolved in the 13.4.4 release, so you have to upgrade to the latest version, I think.

@Afrozefathima
Copy link

Afrozefathima commented Jun 5, 2023

I already upgraded it. Yet not working for me. I ran codemods which added legacy behaviour again but it showed multiple children error and cleared it again manually.

@mhesham32
Copy link

this problem still exists if I tried to navigate using router.push() . I'm using not-found in case my api/route returned an empty result but then if the endpoint returned results the not-found page should disappear and the results should be displayed instead.

if(results === null) return notFound();

... display results

is this a good way to use not-found page?

hydRAnger pushed a commit to hydRAnger/next.js that referenced this issue Jun 12, 2023
## What?

Currently the error boundary state does not reset when a navigation
happens because the `key` does not change. Because of that navigating
using next/link when the error boundary is in the open state ends up not
rendering the contents of the new navigation, instead it keeps the error
boundary contents.

## How?

This PR uses a pattern that is recommended in the React docs (setState
during render) to reset the error boundary if the `usePathname` value
has changed, it is used as a proxy for a new navigation.

Fixes vercel#47862
Fixes NEXT-963
Fixes vercel#49736
Fixes vercel#49732
Fixes vercel#49599


<!-- 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:

## For Contributors

### Improving Documentation or adding/fixing Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

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



## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->
@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 Jul 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: app App directory (appDir: true) linear: next Confirmed issue that is tracked by the Next.js team. locked
Projects
None yet
Development

Successfully merging a pull request may close this issue.