Skip to content

Conversation

@huozhi
Copy link
Member

@huozhi huozhi commented Nov 12, 2025

is-error is a helper that being used accorss client and server now, in #84909 we switched the underlying safe stringify error implementation to the precompiled dependency safe-stable-stringify. But it increased the client bundle size a bit.

The change in #84909 was only for applying the safe-stable-stringify for client file logger with mcp, not really necessary need to change for is-error helper

Note: Save minified around 12kb. it's actually 7.5kb on bundlephobia, but because we're using ncc to compile it to cjs dependency that the size seems slightly larger than their metrics. The saved gzip size would be around 2-3.x kb

@huozhi huozhi marked this pull request as ready for review November 12, 2025 20:20
@huozhi huozhi requested a review from timneutkens November 12, 2025 20:24
@ijjk
Copy link
Member

ijjk commented Nov 12, 2025

Failing test suites

Commit: 43de3e8 | About building and testing Next.js

pnpm test-start-turbo test/production/next-server-nft/next-server-nft.test.ts (turbopack) (job)

  • next-server-nft > should not trace too many files in next-server.js.nft.json (DD)
  • next-server-nft > should not trace too many files in next-minimal-server.js.nft.json (DD)
Expand output

● next-server-nft › should not trace too many files in next-server.js.nft.json

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `next-server-nft should not trace too many files in next-server.js.nft.json 1`

- Snapshot  - 1
+ Received  + 0

@@ -38,11 +38,10 @@
    "/node_modules/next/dist/compiled/path-to-regexp/index.js",
    "/node_modules/next/dist/compiled/picomatch/index.js",
    "/node_modules/next/dist/compiled/react-is/cjs/react-is.development.js",
    "/node_modules/next/dist/compiled/react-is/cjs/react-is.production.js",
    "/node_modules/next/dist/compiled/react-is/index.js",
-   "/node_modules/next/dist/compiled/safe-stable-stringify/index.js",
    "/node_modules/next/dist/compiled/send/index.js",
    "/node_modules/next/dist/compiled/source-map/source-map.js",
    "/node_modules/next/dist/compiled/stacktrace-parser/stack-trace-parser.cjs.js",
    "/node_modules/next/dist/compiled/string-hash/index.js",
    "/node_modules/next/dist/compiled/strip-ansi/index.js",

  114 |       ]
  115 |
> 116 |       expect(traceGrouped).toMatchInlineSnapshot(`
      |                            ^
  117 |        [
  118 |          "/node_modules/@img/colour/*",
  119 |          "/node_modules/@img/sharp-*/sharp-*.node",

  at Object.toMatchInlineSnapshot (production/next-server-nft/next-server-nft.test.ts:116:28)

● next-server-nft › should not trace too many files in next-minimal-server.js.nft.json

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `next-server-nft should not trace too many files in next-minimal-server.js.nft.json 1`

- Snapshot  - 1
+ Received  + 0

@@ -3,11 +3,10 @@
    "/node_modules/next/dist/client/components/app-router-headers.js",
    "/node_modules/next/dist/compiled/@opentelemetry/api/index.js",
    "/node_modules/next/dist/compiled/babel-code-frame/index.js",
    "/node_modules/next/dist/compiled/babel/code-frame.js",
    "/node_modules/next/dist/compiled/next-server/server.runtime.prod.js",
-   "/node_modules/next/dist/compiled/safe-stable-stringify/index.js",
    "/node_modules/next/dist/compiled/source-map/source-map.js",
    "/node_modules/next/dist/compiled/stacktrace-parser/stack-trace-parser.cjs.js",
    "/node_modules/next/dist/compiled/ws/index.js",
    "/node_modules/next/dist/experimental/testmode/context.js",
    "/node_modules/next/dist/experimental/testmode/fetch.js",

  263 |         '.next/next-minimal-server.js.nft.json'
  264 |       )
> 265 |       expect(trace).toMatchInlineSnapshot(`
      |                     ^
  266 |        [
  267 |          "/node_modules/client-only/index.js",
  268 |          "/node_modules/next/dist/client/components/app-router-headers.js",

  at Object.toMatchInlineSnapshot (production/next-server-nft/next-server-nft.test.ts:265:21)

pnpm test-start test/e2e/app-dir/app-prefetch/prefetching.test.ts (job)

  • app dir - prefetching > should show layout eagerly when prefetched with loading one level down (DD)
Expand output

● app dir - prefetching › should show layout eagerly when prefetched with loading one level down

thrown: "Exceeded timeout of 120000 ms for a test.
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."

  26 |   })
  27 |
> 28 |   it('should show layout eagerly when prefetched with loading one level down', async () => {
     |   ^
  29 |     let act: ReturnType<typeof createRouterAct>
  30 |     const timeController = createTimeController()
  31 |     const browser = await next.browser('/', {

  at it (e2e/app-dir/app-prefetch/prefetching.test.ts:28:3)
  at Object.describe (e2e/app-dir/app-prefetch/prefetching.test.ts:11:1)

@huozhi huozhi requested a review from gaojude November 13, 2025 00:36
@huozhi huozhi enabled auto-merge (squash) November 13, 2025 00:40
@huozhi huozhi disabled auto-merge November 13, 2025 01:14
@huozhi huozhi merged commit 1f0322f into canary Nov 13, 2025
400 of 414 checks passed
@huozhi huozhi deleted the huozhi/remove-stringify-dep branch November 13, 2025 01:14
huozhi added a commit that referenced this pull request Nov 13, 2025
`is-error` is a helper that being used accorss client and server now, in
#84909 we switched the underlying safe stringify error implementation to
the precompiled dependency `safe-stable-stringify`. But it increased the
client bundle size a bit.

The change in #84909 was only for applying the `safe-stable-stringify`
for client file logger with mcp, not really necessary need to change for
is-error helper

Note: Save minified around 12kb. it's actually 7.5kb on bundlephobia,
but because we're using ncc to compile it to cjs dependency that the
size seems slightly larger than their metrics. The saved gzip size would
be around 2-3.x kb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants