### What
When there's a same error throwing constantly, we should avoid unnecessary re-renders on the exact same error. In dev overlay we already have strategies that can dedupe the error. And we noticed the error is shown correctly but just the dialog got re-rendered with animations.
After digging deeper, noticed we are still adding the same runtime error into the error collection, just because each time it got the new "ID", as the "ID" itself is incremental based on actions. Each action will get a new error ID. Hence we're adding a condition in deduping. If we found that the new error already existed in the previous events, we'll return with the same previous events instance to tell dev overlay that errors are not changed.
Fixes NEXT-4493
#### After
https://github.com/user-attachments/assets/c1f004b5-2de6-445a-aad0-efdfd0ecb8c9
#### Before
https://github.com/user-attachments/assets/1249ed54-ba63-4eb6-a08a-64560ab4014a
When turbopack reports an issue, compute a set of import-traces for that file so the user can better understand the context of the issue and trivially answer questions like:
* Why is this file included at all?
* Why does next think this is a client-component?
To do this we leverage the `SingleModuleGraph` and use the `astar` algorithm from `petgraph` to compute the shortest path to a root module. This isn't the most optimal approach but should be sufficient since we don't anticipate this being a performance issue.
A complex part of this is that the module-graph tracks the relationships between _modules_ but Issues are associated with _files_. While modules are also associated with files this is a many-to-one relationship. This is why we might report _multiple_ traces for a single issue and also why a single file might appear multiple times in a trace.
## Open formatting questions
* how should we represent paths from other 'filesystems'?
- for disk filesystems i could compute relative paths to the root of the current directory? the `[project]` filesystem? For now i just use the filesystem name as a hypothetically cromulent root.
## Alternatives
The main alternative investigated was associating Issues with `Modules` by collecting them during graph construction. This unfortunately proved to be a non-trivial performance regression and so it was abandoned. The core problem is that we would need to introduce additional `OperationVc` and task roots to simply `collect` the issues. This also wouldn't eliminate the duplicate traces issue, and instead we might end up reporting duplicate issues instead.
## Performance
TODO
Closes PACK-4105
May have been a leftover from earlier.
We always consider any throw. Even without a sync stack, we might still have an async stack to show. And even if we have no stack at all, the message is still relevant. Basically, never hide errors.
This updates the dynamic validation implementation to more closely
follow the pattern used in prerenders for dynamicIO + PPR. Now that PPR
is the only way to use dynamicIO we only have one implementation and the
validation should match it as much as possible.
To prepare for an upcoming change where `ppr` will be enabled
automatically when `dynamicIO` is enabled, we're hard coding `ppr: true`
in the Dynamic IO tests with this PR. The non-PPR assertions are
subsequently removed, which makes up most of the changed lines.
We can now also exclude the tests in the PPR tests manifest, so that
they're not run twice in CI.
> [!NOTE]
> This PR is best reviewed with hidden whitespace changes.
_Relanding #79151, now ignoring TS AST nodes when checking whether all
exported module items are async functions._
This fixes an error where we incorrectly showed the following compiler
error when using Turbopack:
```sh
Ecmascript file had an error
> 1 | 'use cache'
| ^^^^^^^^^^^
2 |
3 | import { useStuff } from './client-module'
4 |
The "use cache" directive must be at the top of the file.
```
The directive was regarded as not being at the top of the file, because
the React transform (`EcmascriptInputTransform::React`) injects a
variable declaration for React Refresh (e.g. `var _s =
__turbopack_context__.k.signature`) at the top of the file, before the
`"use cache"` directive.
We can fix this by ensuring that the server action transform (should
really be called "server _function_ transform") runs first.
As a consequence, the transform now needs to be able to parse JSX AST
nodes, which was already explicitly implemented.
closes NAR-130
<!-- 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
- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide
### Adding or Updating 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 #
-->
Reverts vercel/next.js#79151
It caused a regression like this
```
export type Foo =
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Only async functions are allowed to be exported in a "use server" file.
```
This fixes an error where we incorrectly showed the following compiler
error when using Turbopack:
```sh
Ecmascript file had an error
> 1 | 'use cache'
| ^^^^^^^^^^^
2 |
3 | import { useStuff } from './client-module'
4 |
The "use cache" directive must be at the top of the file.
```
The directive was regarded as not being at the top of the file, because
the React transform (`EcmascriptInputTransform::React`) injects a
variable declaration for React Refresh (e.g. `var _s =
__turbopack_context__.k.signature`) at the top of the file, before the
`"use cache"` directive.
We can fix this by ensuring that the server action transform (should
really be called "server _function_ transform") runs first.
As a consequence, the transform now needs to be able to parse JSX AST
nodes, which was already explicitly implemented.
closes NAR-130
While working on filesystem watching stuff, I saw that `notify-rs` had a new `with_follow_symlinks` flag, which seems very relevant to us and how we handle symlinks.
When looking to enable this, I noticed that we lacked e2e tests for symlinks. I added a few tests, but found lots of other bugs...
- I ran into https://github.com/vercel/next.js/discussions/77463#discussioncomment-12847522, which I documented, but am not fixing, because it's too far out of scope.
- I ran into a bug with realpath, which is fixed here.
This test is flaky in headless mode but passing in browser mode. It was
timeout on checking the toast. Adding an assertion for it seems resolve
the issue
When `dynamicIO` is enabled, we're triggering a warmup request in dev mode. This ensures that replayed logs are associated with the correct environment (`Prerender` vs. `Server`), by seeding the caches before the actual render.
This PR fixes two issues with the dev warmup:
- Ensures that cache keys are identical between the warmup, the subsequent dynamic render, and the dynamic validation, by providing the HMR refresh hash (part of the cache key) for all dev render phases.
- Ensures that stale cache entries are discarded during the warmup, by providing the implicit tags also during the warmup (and the dynamic validation).
This is a follow-up fix for #75474.
When switching from `next dev` to `next start` locally, a user might
have an existing HMR refresh hash session cookie.
We need to make sure that the hash value is excluded from any `"use
cache"` cache keys, when running with `next start`. Otherwise, the cache
keys will be different when prerendering vs. handling a dynamic request,
e.g. when executing a server action.
In production, we now also omit the serialized `"$undefined"` value for
the HMR refresh hash from the cache keys to keep them shorter.
Adds a field `navigatedAt` to the CacheNode type. It represents the
timestamp of the navigation that last updated the CacheNode's data.
This will be used to implement the `staleTimes.dynamic` configuration
option, which allows dynamic data to be stale in the UI up to the given
threshold. This first PR adds the field without changing any behavior.
Originally I was going to call this `requestedAt`, but I have
intentionally chosen not to distinguish between whether the data was
fetched from the network or from the prefetch cache. This means that
even fully static data will respect the `staleTimes.dynamic`
configuration option. In practice, this only matters if the dynamic
stale time is longer than the static one, although usually static stale
times will be larger anyway.
### What?
- Added additional debug logging for `customRoutes`,
`publicFolderItems`, and `nextStaticFolderItems` in the filesystem
router utils
- Improved the dynamic route interpolation test to check all script
sources instead of just the first matching one
- Fixed the Turbopack build test manifest to correctly mark the dynamic
route interpolation test as passing
- Updated the `to_string()` implementation in `ServerFileSystem` to use
a hyphenated format so that it behaves the same between urlencoded and
urldecoded.
### Why?
This PR improves debugging capabilities for the filesystem router and
fixes the dynamic route interpolation test to be more robust by checking
all script sources rather than just the first one that matches certain
criteria. The test now properly verifies that both encoded and decoded
paths work correctly.
### How?
- Added debug statements for additional route-related variables
- Refactored the test to collect all script sources and verify each one
works with both encoded and decoded paths
- Updated the Turbopack test manifest to reflect the fixed test
- Changed the server filesystem string representation to use hyphens
instead of spaces
Best to review commit by commit.
The App Router's state lives outside of React, so we don't need to pass
the dispatch method around via props/context. We can import it directly
into each module that needs it.
We have to be a bit clever and assign the dispatch method to a global
variable on initialization. This is more complicated than it should be
because of a known issue that requires us to decode Flight streams
during the render phase.
Ideally, we would store the action queue and dispatch method entirely
outside of React and pass the state into React as a prop using
`root.render`; conceptually, this is how we're modeling it despite the
weird implementation details.
(We were already using this approach in several places, so it's not new
to this PR; I'm just explaining it here for context.)
The first commit moves dispatch to a global. The rest of the PR cleans
up indirections that can be removed by importing and calling the
dispatch method directly.
### What
We had a few existing hacks for the missing html tag errors, we used a
lot of tricks and components to deal with it specifically. It was quite
a pain to handle on specific error across the entire codebase. This PR
leverage the existing SSR error template we sent through the rendering.
We now inject a open `<html>` tag to intercept the HTML. And then it
will be consumed as a SSR error. We added one more thing that we'll open
the error overlay automatically by default. It doesn't have to be build
error since the display was not ideal.
Now it's just one of the normal error, such like it can show the version
indicator as well. Except we had a little bit logic to make it pop up by
default.
The idea is to insert a piece of errored html like what we expect for
erroring rendered html. So it inserts a new start point of the html, and
then React will hydrate it with the error template.
```html
<!-- layout / page content -->
<meta .. />
<script .. />
<!-- we insert a html for missing tag error -->
<html id="__next_error__">
<template data-next-error-message=".." ..></template>
<!-- next.js always render close tag -->
</body>
</html>
```
| After | Before |
|:-- |:--|
| <img width="1048" alt="image"
src="https://github.com/user-attachments/assets/713812cf-50cd-4b14-8906-44ed3795ef12"
/> | <img width="1020" alt="image"
src="https://github.com/user-attachments/assets/ac7614d9-da3b-48d1-bab1-c97355c27963"
/> |
The current animation strategy for our Dev Tools surface does not work.
https://github.com/user-attachments/assets/a0ca5459-9d5a-4d90-af00-b071d43581e0
On page load, several animations can be observed:
- Width growth from `0` → `auto`
- Issue count translates and fades, along with the plural `s` suffix
- A subtle scale bounce effect plays
All these animations are state transitions, and thus we should ignore
them completely on page load or on high-frequency updates where
animating is undesirable, e.g. sometimes `issueCount` will receive
updates faster than the animation can run.
This PR implements checks to guard the animations from being executed on
page load, _and only_ during state transitions.
To review, it is worth checking out the
[/animation-test](https://github.com/vercel/next.js/blob/a6e6fe635b712020d4ea2d027040d838eb5941c6/test/development/app-dir/hydration-error-count/app/animation-test/page.tsx)
fixture I've set up in this PR.
After these changes, the experience is as follows—no animations on page
load, but only on subsequent low-frequency updates (count change, or
collapse of the surface):
https://github.com/user-attachments/assets/7b2d5e0d-64ec-4165-b5e2-85489bd00a2c
_Note: I am not using Storybook for a page load reference, the apps
under `test/development/app-dir/*` are more reliable._
### Why?
When users want to hide the dev indicator, they might be frustrated if
it reappears every reload. Hence, make a default to hide it for the
current server session or a day.
Closes NDX-890
---------
Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>
The static indicator was added in the new dev overlay but wasn't hooked up to pages router. This wires that up so they're properly marked static/dynamic.
Closes NDX-897
Jiachi Liu