Fix bug with "Circular Structure" error #23905

Skn0tt · 2021-04-12T08:41:55Z

Since -1 is truthy, every JSON.stringify error is mistaken to be circular structure. This commit fixes that behaviour, so that other errors like Do not know how to serialize Bigint (see blitz-js/babel-plugin-superjson-next#63) aren't swallowed.

Bug

Related issues linked using fixes #number
Integration tests added

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
Integration tests added
Documentation added
Telemetry added. In case of a feature if it's used or not.

Documentation / Examples

Make sure the linting passes

Since `-1` is truthy, every JSON.stringify error is mistaken to be `circular structure`. This commit fixes that behaviour, so that other errors like `Do not know how to serialize Bigint` (see blitz-js/babel-plugin-superjson-next#63) aren't swallowed.

timneutkens

Can you add an integration test for this 👍

This may be thought of as being a pretty contrived example, but it's exactly what happened in blitz-js/babel-plugin-superjson-next#63.

Skn0tt · 2021-04-13T10:38:29Z

test/integration/json-serialize-original-error/pages/bigint.js

+}
+
+export default function Repro(props) {
+  props.topics[0].number = 22n // basically what happened in https://github.com/blitz-js/babel-plugin-superjson-next/issues/63


AFAICT, this error really only occurs when props are mutated during rendering. Normal JSON-misuse (e.g. returning bigints from getStaticProps) is catched before by custom Next.js logic.
Mutating during render is a big no-no, so errors that aren't about "circular structure" could even be used to indicate the mistake and point towards the React docs.

ijjk · 2021-04-13T10:45:33Z

Stats from current PR

Default Server Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
buildDuration	13.9s	13.8s	-91ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +24 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.527	2.48	-0.05
/ avg req/sec	989.38	1007.99	+18.61
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.277	1.33	⚠️ +0.05
/error-in-render avg req/sec	1958.11	1879.54	⚠️ -78.57

Client Bundles (main, webpack, commons)

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
597-3f457946..288d.js gzip	13.3 kB	13.3 kB	✓
778-6834d41c..48a9.js gzip	7.05 kB	7.05 kB	✓
framework.HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	151 B	151 B	✓
webpack-HASH.js gzip	993 B	993 B	✓
Overall change	60.8 kB	60.8 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
_app-5cc66b2..6f03.js gzip	1.3 kB	1.3 kB	✓
_error-55112..054d.js gzip	3.68 kB	3.68 kB	✓
amp-89a5460c..567f.js gzip	558 B	558 B	✓
hooks-8c2e74..be37.js gzip	924 B	924 B	✓
index-fec729..83b2.js gzip	243 B	243 B	✓
link-dd34d9b..0ade.js gzip	1.66 kB	1.66 kB	✓
routerDirect..5759.js gzip	336 B	336 B	✓
withRouter-1..98bf.js gzip	334 B	334 B	✓
Overall change	9.03 kB	9.03 kB	✓

Client Build Manifests

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
_buildManifest.js gzip	349 B	349 B	✓
Overall change	349 B	349 B	✓

Rendered Page Sizes

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
index.html gzip	610 B	610 B	✓
link.html gzip	617 B	617 B	✓
withRouter.html gzip	605 B	605 B	✓
Overall change	1.83 kB	1.83 kB	✓

Serverless Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
buildDuration	17.4s	15.9s	-1.5s
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +24 B

Client Bundles (main, webpack, commons)

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
597-3f457946..288d.js gzip	13.3 kB	13.3 kB	✓
778-6834d41c..48a9.js gzip	7.05 kB	7.05 kB	✓
framework.HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	151 B	151 B	✓
webpack-HASH.js gzip	993 B	993 B	✓
Overall change	60.8 kB	60.8 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
_app-5cc66b2..6f03.js gzip	1.3 kB	1.3 kB	✓
_error-55112..054d.js gzip	3.68 kB	3.68 kB	✓
amp-89a5460c..567f.js gzip	558 B	558 B	✓
hooks-8c2e74..be37.js gzip	924 B	924 B	✓
index-fec729..83b2.js gzip	243 B	243 B	✓
link-dd34d9b..0ade.js gzip	1.66 kB	1.66 kB	✓
routerDirect..5759.js gzip	336 B	336 B	✓
withRouter-1..98bf.js gzip	334 B	334 B	✓
Overall change	9.03 kB	9.03 kB	✓

Client Build Manifests

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
_buildManifest.js gzip	349 B	349 B	✓
Overall change	349 B	349 B	✓

Serverless bundles Overall increase ⚠️

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
_error.js	1.35 MB	1.35 MB	⚠️ +7 B
404.html	2.76 kB	2.76 kB	✓
500.html	2.75 kB	2.75 kB	✓
amp.amp.html	10.7 kB	10.7 kB	✓
amp.html	1.96 kB	1.96 kB	✓
hooks.html	2.01 kB	2.01 kB	✓
index.js	1.35 MB	1.35 MB	⚠️ +7 B
link.js	1.4 MB	1.4 MB	⚠️ +7 B
routerDirect.js	1.4 MB	1.4 MB	⚠️ +7 B
withRouter.js	1.4 MB	1.4 MB	⚠️ +7 B
Overall change	6.91 MB	6.91 MB	⚠️ +35 B

Webpack 5 Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
buildDuration	12.4s	12.7s	⚠️ +226ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +24 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.261	2.294	⚠️ +0.03
/ avg req/sec	1105.63	1089.63	⚠️ -16
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.112	1.161	⚠️ +0.05
/error-in-render avg req/sec	2248.26	2152.5	⚠️ -95.76

Client Bundles (main, webpack, commons)

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
597-3f457946..288d.js gzip	13.3 kB	13.3 kB	✓
778-6834d41c..48a9.js gzip	7.05 kB	7.05 kB	✓
framework.HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	151 B	151 B	✓
webpack-HASH.js gzip	993 B	993 B	✓
Overall change	60.8 kB	60.8 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
_app-5cc66b2..6f03.js gzip	1.3 kB	1.3 kB	✓
_error-55112..054d.js gzip	3.68 kB	3.68 kB	✓
amp-89a5460c..567f.js gzip	558 B	558 B	✓
hooks-8c2e74..be37.js gzip	924 B	924 B	✓
index-fec729..83b2.js gzip	243 B	243 B	✓
link-dd34d9b..0ade.js gzip	1.66 kB	1.66 kB	✓
routerDirect..5759.js gzip	336 B	336 B	✓
withRouter-1..98bf.js gzip	334 B	334 B	✓
Overall change	9.03 kB	9.03 kB	✓

Client Build Manifests

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
_buildManifest.js gzip	349 B	349 B	✓
Overall change	349 B	349 B	✓

Rendered Page Sizes

	vercel/next.js canary	Skn0tt/next.js patch-1	Change
index.html gzip	610 B	610 B	✓
link.html gzip	617 B	617 B	✓
withRouter.html gzip	605 B	605 B	✓
Overall change	1.83 kB	1.83 kB	✓

Diffs

Diff for index.html

@@ -48,7 +48,7 @@
         "props": { "pageProps": {} },
         "page": "/",
         "query": {},
-        "buildId": "AUTQttZNxUUmJwQ4PzaK_",
+        "buildId": "Iu19PHIGbHowJ8MotdiUd",
         "isFallback": false,
         "gip": true
       }
@@ -86,11 +86,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/AUTQttZNxUUmJwQ4PzaK_/_buildManifest.js"
+      src="/_next/static/Iu19PHIGbHowJ8MotdiUd/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/AUTQttZNxUUmJwQ4PzaK_/_ssgManifest.js"
+      src="/_next/static/Iu19PHIGbHowJ8MotdiUd/_ssgManifest.js"
       async=""
     ></script>
   </body>

Diff for link.html

@@ -53,7 +53,7 @@
         "props": { "pageProps": {} },
         "page": "/link",
         "query": {},
-        "buildId": "AUTQttZNxUUmJwQ4PzaK_",
+        "buildId": "Iu19PHIGbHowJ8MotdiUd",
         "isFallback": false,
         "gip": true
       }
@@ -91,11 +91,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/AUTQttZNxUUmJwQ4PzaK_/_buildManifest.js"
+      src="/_next/static/Iu19PHIGbHowJ8MotdiUd/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/AUTQttZNxUUmJwQ4PzaK_/_ssgManifest.js"
+      src="/_next/static/Iu19PHIGbHowJ8MotdiUd/_ssgManifest.js"
       async=""
     ></script>
   </body>

Diff for withRouter.html

@@ -48,7 +48,7 @@
         "props": { "pageProps": {} },
         "page": "/withRouter",
         "query": {},
-        "buildId": "AUTQttZNxUUmJwQ4PzaK_",
+        "buildId": "Iu19PHIGbHowJ8MotdiUd",
         "isFallback": false,
         "gip": true
       }
@@ -86,11 +86,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/AUTQttZNxUUmJwQ4PzaK_/_buildManifest.js"
+      src="/_next/static/Iu19PHIGbHowJ8MotdiUd/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/AUTQttZNxUUmJwQ4PzaK_/_ssgManifest.js"
+      src="/_next/static/Iu19PHIGbHowJ8MotdiUd/_ssgManifest.js"
       async=""
     ></script>
   </body>

Commit: 4ecefb7

# Conflicts: # packages/next/pages/_document.tsx

ijjk

Thanks for the PR!

test added

Skn0tt requested review from divmain, ijjk, lfades, shuding and timneutkens as code owners April 12, 2021 08:41

ijjk added the type: next label Apr 12, 2021

timneutkens previously requested changes Apr 12, 2021

View reviewed changes

Add integration test

4ecefb7

This may be thought of as being a pretty contrived example, but it's exactly what happened in blitz-js/babel-plugin-superjson-next#63.

Skn0tt commented Apr 13, 2021

View reviewed changes

Skn0tt requested a review from timneutkens April 13, 2021 15:20

Skn0tt requested review from huozhi, padmaia and styfle as code owners October 26, 2021 15:29

Merge branch 'canary' into patch-1

1138a3b

# Conflicts: # packages/next/pages/_document.tsx

ijjk approved these changes Feb 10, 2022

View reviewed changes

Merge branch 'canary' into patch-1

ab71b53

ijjk merged commit abf9f75 into vercel:canary Feb 10, 2022

github-actions bot locked as resolved and limited conversation to collaborators Mar 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bug with "Circular Structure" error #23905

Fix bug with "Circular Structure" error #23905

Skn0tt commented Apr 12, 2021

timneutkens left a comment

Skn0tt Apr 13, 2021 •

edited

ijjk commented Apr 13, 2021

Diffs

ijjk left a comment

Fix bug with "Circular Structure" error #23905

Fix bug with "Circular Structure" error #23905

Conversation

Skn0tt commented Apr 12, 2021

Bug

Feature

Documentation / Examples

timneutkens left a comment

Choose a reason for hiding this comment

Skn0tt Apr 13, 2021 • edited

Choose a reason for hiding this comment

ijjk commented Apr 13, 2021

Stats from current PR

Diffs

ijjk left a comment

Choose a reason for hiding this comment

Skn0tt Apr 13, 2021 •

edited