sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

React Hooks issue with react-router-v6 integration

Open alexabidri opened this issue 3 years ago • 5 comments

Is there an existing issue for this?

  • [X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
  • [X] I have reviewed the documentation https://docs.sentry.io/
  • [X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/react

SDK Version

7.2.0

Framework Version

7.2.0

Steps to Reproduce

1. Starting a react application with Vite.js in dev mode with react-refresh 2. Adding a simple Application like this one

import React from 'react';
import { createRoot } from 'react-dom/client';
import { BrowserRouter, useLocation, useNavigationType, createRoutesFromChildren, matchRoutes, Routes, Route } from 'react-router-dom';
import * as Sentry from '@sentry/react';
import { BrowserTracing } from '@sentry/tracing';
import { APP_ENVIRONMENT, SENTRY_DSN } from './config';

Sentry.init({
    dsn: SENTRY_DSN,
    integrations: [
        new BrowserTracing({
            routingInstrumentation: Sentry.reactRouterV6Instrumentation(React.useEffect, useLocation, useNavigationType, createRoutesFromChildren, matchRoutes),
        }),
    ],
    environment: APP_ENVIRONMENT,
    tracesSampleRate: 1.0,
});

const SentryRoutes = Sentry.withSentryReactRouterV6Routing(Routes);


function render() {
    const container = document.getElementById('root');
    if (!container) {
        return null;
    }
    const root = createRoot(container);
    root.render(
        <React.StrictMode>
            <BrowserRouter>
                <SentryRoutes>
                    <Route path="/" element={<React.Fragment />} />
                </SentryRoutes>
            </BrowserRouter>
        </React.StrictMode>,
    );
}

(() => {
    render();
})();

Same code is in the official doc of Sentry https://docs.sentry.io/platforms/javascript/guides/react/configuration/integrations/react-router/ 3. The app starts normally in dev mode. 4. As soon as I perform one modification, the front-end application crash completely. I need to close the tab and stop the application.

When I remove

new BrowserTracing({
            routingInstrumentation: Sentry.reactRouterV6Instrumentation(React.useEffect, useLocation, useNavigationType, createRoutesFromChildren, matchRoutes)
})

from the sentry integrations, everything goes back to normal

Expected Result

When I do a change in local while I am developing, I should see my changes in live without my app crashing. I should be able to use BrowserTracing with Sentry without my app having issues

Actual Result

Warning: React has detected a change in the order of Hooks called by SentryRoutes. This will lead to bugs and errors if not fixed. For more information, read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks

There is issue in the order of hooks execution while adding the BrowserTracing from Sentry. Screenshot 2022-06-24 at 18 19 44

Vite.js config to start the app.

import { defineConfig, loadEnv } from 'vite';
import { visualizer } from 'rollup-plugin-visualizer';
import react from '@vitejs/plugin-react';
import svgrPlugin from 'vite-plugin-svgr';

export default defineConfig(({ mode }) => {
    const env = loadEnv(mode, process.cwd());

    // expose .env as process.env instead of import.meta since jest does not import meta yet
    const envWithProcessPrefix = Object.entries(env).reduce((prev, [key, val]) => {
        return {
            ...prev,
            ['process.env.' + key]: `"${val}"`,
        };
    }, {});

    return {
        server: {
            port: 3000,
        },
        preview: {
            port: 8080,
        },
        define: envWithProcessPrefix,
        build: {
            sourcemap: true,
            outDir: 'build',
            minify: 'esbuild',
        },
        plugins: [
            react(),
            svgrPlugin({
                svgrOptions: {
                    // ...svgr options (https://react-svgr.com/docs/options/)
                },
            }),
            visualizer(),
        ],
    };
});

alexabidri avatar Jun 24 '22 10:06 alexabidri

@onurtemizkan could you take a look at this?

AbhiPrasad avatar Jun 24 '22 18:06 AbhiPrasad

Thanks for reporting this @alexabidri.

I can't reproduce this on a clean Vite.js project using the configuration / example above. Could you provide a reproduction repo for us to work on please?

onurtemizkan avatar Jun 27 '22 13:06 onurtemizkan

@onurtemizkan No problem, here it is https://github.com/alexabidri/vite-sentry 🙏

1 - Start the app with npm run dev 2 - Modify the wording TEST into something else here https://github.com/alexabidri/vite-sentry/blob/main/src/App.tsx#L11, it will trigger react hot reloading 3 - Your app will crash and the browser also with, with a issue related to Sentry React Router v6 integration (hooks issue)

alexabidri avatar Jun 30 '22 03:06 alexabidri

I can confirm same problem with react without Vite (react create app). @sentry/react 7.8.1 @sentry/tracing 7.8.1 react 18.2.0 react-router-dom 6.3.0

jandominik avatar Aug 03 '22 19:08 jandominik

@jandominik could you try passing debug: true to your Sentry.init call and seeing what the SDK logs out?

This is how we define SentryRoutes. Not sure why it's causing issues - we do any conditional calls of hooks: https://github.com/getsentry/sentry-javascript/blob/31fd072e0390025ed818301bc212528a51e1b502/packages/react/src/reactrouterv6.tsx#L130-L180

Maybe this is an issue with Hot Module Reloading?

We've backlogged this for now - but PRs/debugging is welcome if anyone can take a look!

AbhiPrasad avatar Aug 03 '22 19:08 AbhiPrasad

Hey @alexabidri,

After debugging the reproduction, I was able to fix the issue by initializing Sentry at App.tsx instead of main.tsx.

The problem is, the routes were never actually wrapped, because Sentry.withSentryReactRouterV6Routing was called before the initialization (App.tsx runs before main.tsx).

If HMR refreshes App.tsx, it tries to wrap Routes again, and succeeds this time, because the initialization is already happened. But it conflicts, as the hook order of the wrapped Routes is not matching the non-wrapped one, which is already rendered.

Not sure if we can do anything on the SDK side. But I'll add a note to the docs, reminding users to make sure init runs before the wrapper.

onurtemizkan avatar Sep 01 '22 12:09 onurtemizkan

Silly me ran into this by having this outside my functional component: const SentryRoutes = Sentry.withSentryReactRouterV6Routing(Routes);

Moving that to the top level, within the function, resolved this issue.

mswezey23 avatar Oct 26 '22 20:10 mswezey23