fix(react): Support lazy-loaded routes and components. by onurtemizkan · Pull Request #15039 · getsentry/sentry-javascript

onurtemizkan · 2025-01-16T22:26:35Z

This PR adds support for lazily loaded components and routes inside Suspend on react-router pageloads / navigations.

mydea · 2025-01-17T07:27:43Z

dev-packages/e2e-tests/test-applications/react-create-browser-router-lazy-routes/src/index.tsx

+    },
+    {
+      element: (
+        <Suspense fallback={<div>Loading...</div>}>


No strong feelings, this is also fine, but could we possibly add this to an existing e2e test app? Would save a little but of ci/processing time 😅

We could add this to react-create-browser-router :)

mydea · 2025-01-17T07:29:03Z

packages/react/src/reactrouterv6-compat-utils.tsx

-          version,
-          basename,
+        // Use requestAnimationFrame to wait for Suspense boundaries to settle
+        requestAnimationFrame(() => {


this will slightly skew all timestamps, even if there is no suspense etc. happening, right? As this will realistically add ~20ms or so before handleNavigation() is called.

Not a blocker IMHO, but something to consider. Can we make this smarter (e.g. know when this is lazy?) somehow, possibly...?

mydea · 2025-01-17T07:30:02Z

packages/react/src/reactrouterv6-compat-utils.tsx

+              version,
+              basename,
+            });
+          }, 100);


100ms delay seems quite a lot, this will skew stuff pretty considerably, I guess 😬 does that not add 100ms to every navigation duration etc...?

I think we could maybe look into other fields of the RouterState https://github.com/remix-run/react-router/blob/d0e474cf6c521881044c445b4730c1a43aa77679/packages/react-router/lib/router/router.ts#L276

E.g. call handleNavigation when state.navigation !== 'loading'?

Yes, updated with the navigation state checker 👍 I also realized that putting the manual timeout made us lose the inner resource.

codecov · 2025-01-17T12:57:45Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
689	1	688	299

View the full list of 1 ❄️ flaky tests

tracing/request/fetch/test.ts should create spans for fetch requests
Flake rate in main: 14.29% (Passed 54 times, Failed 9 times)
Stack Traces | 10.1s run time
test.ts:7:11 should create spans for fetch requests

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

s1gr1d · 2025-01-17T13:15:03Z

packages/react/src/reactrouterv6-compat-utils.tsx

-        });
+        // Wait for the next render if loading an unsettled route
+        if (state.navigation.state !== 'idle') {
+          requestAnimationFrame(() => {


Oh, my idea with requestAnimationFrame actually worked - that's nice ✨
Thanks for implementing 🙌

But as Francesco pointed out - we should keep in mind the slight overhead and maybe there's even another way to implement this 🤔 Do we know if a route is lazy?

s1gr1d · 2025-01-17T13:17:33Z

dev-packages/e2e-tests/test-applications/react-create-browser-router-lazy-routes/src/index.tsx

+    },
+    {
+      element: (
+        <Suspense fallback={<div>Loading...</div>}>


We could add this to react-create-browser-router :)

github-actions · 2025-01-20T16:45:44Z

size-limit report 📦

Path	Size	% Change	Change
@sentry/browser	22.98 KB	-	-
@sentry/browser - with treeshaking flags	21.64 KB	-	-
@sentry/browser (incl. Tracing)	35.68 KB	-	-
@sentry/browser (incl. Tracing, Replay)	72.47 KB	-	-
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags	62.98 KB	-	-
@sentry/browser (incl. Tracing, Replay with Canvas)	76.72 KB	-	-
@sentry/browser (incl. Tracing, Replay, Feedback)	88.74 KB	-	-
@sentry/browser (incl. Feedback)	39.2 KB	-	-
@sentry/browser (incl. sendFeedback)	27.61 KB	-	-
@sentry/browser (incl. FeedbackAsync)	32.37 KB	-	-
@sentry/react	25.66 KB	-	-
@sentry/react (incl. Tracing)	38.46 KB	+0.02%	+6 B 🔺
@sentry/vue	27.04 KB	-	-
@sentry/vue (incl. Tracing)	37.43 KB	-	-
@sentry/svelte	23.11 KB	-	-
CDN Bundle	24.36 KB	-	-
CDN Bundle (incl. Tracing)	36 KB	-	-
CDN Bundle (incl. Tracing, Replay)	70.65 KB	-	-
CDN Bundle (incl. Tracing, Replay, Feedback)	75.79 KB	-	-
CDN Bundle - uncompressed	71.16 KB	-	-
CDN Bundle (incl. Tracing) - uncompressed	106.82 KB	-	-
CDN Bundle (incl. Tracing, Replay) - uncompressed	217.67 KB	-	-
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed	230.21 KB	-	-
@sentry/nextjs (client)	38.58 KB	-	-
@sentry/sveltekit (client)	36.21 KB	-	-
@sentry/node	161.33 KB	-	-
@sentry/node - without tracing	97.15 KB	-	-
@sentry/aws-serverless	111.45 KB	-	-

View base workflow run

onurtemizkan · 2025-01-21T08:30:18Z

@mydea, @s1gr1d, @chargome - I updated the PR to cover the lazy-loaded Routes too. So the problem was the cross-usage of wrapCreateBrowser and withSentryReactRouterRouting. We were losing the allRoutes context in one another and that was why we were not creating the full parameterized span name.

We're actually updating pageload spans and handling navigations on their render, so we can expect the lazy loading to be finished when we do them.

Still, I think we can keep requestAnimationFrame for non-idle router states when we do it as a secondary safety net. But interestingly, I can't reproduce a case with a non-idle state anymore. Looking at the RR source code, it seems possible.

s1gr1d

Looks good from my side

mydea

nice, this looks neat - great work!

Fixes: #15027 This PR adds support for lazily loaded components and routes inside `Suspend` on react-router pageloads / navigations.

Backports #15039 to v8 branch Potentially fixes as it also fixes cross-usage of `createBrowserRouter` and `useRoutes`: #15279

fix(react): Wait for lazy-loaded pages on navigation

acf0082

onurtemizkan requested review from chargome and s1gr1d January 16, 2025 22:26

onurtemizkan mentioned this pull request Jan 16, 2025

React Router browser tracing - Lazy imported routes with suspense start transaction spans with wrong path #15027

Closed

3 tasks

onurtemizkan changed the title ~~fix(react): Wait for lazy-loaded pages on navigation~~ fix(react): Wait for lazy-loaded components on navigation Jan 16, 2025

mydea reviewed Jan 17, 2025

View reviewed changes

Conditionally wait for the render depending on the router state.

1e023fb

s1gr1d reviewed Jan 17, 2025

View reviewed changes

Support lazy-loaded descendant routes.

2a8888d

onurtemizkan force-pushed the onur/rr-lazy-loaded-pages branch from ea60f3a to 2a8888d Compare January 20, 2025 16:46

s1gr1d approved these changes Jan 21, 2025

View reviewed changes

onurtemizkan changed the title ~~fix(react): Wait for lazy-loaded components on navigation~~ fix(react): Support lazy-loaded routes and components. Jan 21, 2025

chargome approved these changes Jan 22, 2025

View reviewed changes

mydea approved these changes Jan 23, 2025

View reviewed changes

s1gr1d merged commit b49c1cc into develop Jan 23, 2025
127 checks passed

s1gr1d deleted the onur/rr-lazy-loaded-pages branch January 23, 2025 08:43

onurtemizkan mentioned this pull request Jan 23, 2025

React Router - Support Lazy-Loaded Routes #15040

Closed

onurtemizkan added a commit that referenced this pull request Feb 3, 2025

fix(react): Support lazy-loaded routes and components. (#15039)

60ac908

Fixes: #15027 This PR adds support for lazily loaded components and routes inside `Suspend` on react-router pageloads / navigations.

onurtemizkan mentioned this pull request Feb 3, 2025

fix(react/v8): Support lazy-loaded routes and components #15281

Merged

s1gr1d pushed a commit that referenced this pull request Feb 11, 2025

fix(react/v8): Support lazy-loaded routes and components (#15281)

6e996e0

Backports #15039 to v8 branch Potentially fixes as it also fixes cross-usage of `createBrowserRouter` and `useRoutes`: #15279

Uh oh!

Conversation

onurtemizkan commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mydea Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

s1gr1d Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

mydea Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

mydea Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

chargome Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

onurtemizkan Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 1 Tests Failed:

Uh oh!

s1gr1d Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

s1gr1d Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

onurtemizkan commented Jan 21, 2025

Uh oh!

s1gr1d left a comment

Choose a reason for hiding this comment

Uh oh!

mydea left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

onurtemizkan commented Jan 16, 2025 •

edited

Loading

codecov bot commented Jan 17, 2025 •

edited

Loading

s1gr1d Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 20, 2025 •

edited

Loading