feat: Add wizard for react-router framework mode
#1076
Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1076 +/- ##
==========================================
+ Coverage 32.06% 32.96% +0.90%
==========================================
Files 133 141 +8
Lines 15638 16802 +1164
Branches 1093 1205 +112
==========================================
+ Hits 5014 5539 +525
- Misses 10607 11244 +637
- Partials 17 19 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
onurtemizkan
force-pushed
the
onur/reactrouter-framework-wizard
branch
from
e0a23a8 to
9e3cca6
Compare
onurtemizkan
force-pushed
the
onur/reactrouter-framework-wizard
branch
from
1baebe3 to
80a9605
Compare
e2e-tests/tests/react-router.test.ts
Outdated
| tracesSampleRate: 1, | ||
| enableLogs: true, | ||
|
|
||
| integrations: [browserTracingIntegration({ |
There was a problem hiding this comment.
q: Why are we expecting the browserTracingIntegration? I don't see this in the docs.
| ); | ||
| } catch (e) { | ||
| clack.log.warn( | ||
| `Could not initialize Sentry on client entry.\n Please do it manually using instructions from https://docs.sentry.io/platforms/javascript/guides/react-router/`, |
There was a problem hiding this comment.
m: In the other wizard we print what users have to add if steps fail. We should do the same for all of these.
src/react-router/sdk-setup.ts
Outdated
| clack.log.warn( | ||
| `Could not find ${chalk.cyan( | ||
| clientEntryFilename, | ||
| )}. Skipping client entry instrumentation.`, | ||
| ); |
There was a problem hiding this comment.
m: I think we could ask users if they want to try rerunning reveal and if that still didn't work then print out what they have to add to the entry.client file.
src/react-router/sdk-setup.ts
Outdated
| await showCopyPasteInstructions({ | ||
| filename: clientEntryFilename, | ||
| codeSnippet: sentryInitCode, | ||
| hint: 'Add this code at the top of your client entry file', | ||
| }); |
There was a problem hiding this comment.
Let's move this into the outer catch and just rethrow here so we always get these instructions when having to paste.
src/react-router/sdk-setup.ts
Outdated
| clack.log.warn( | ||
| `Could not find ${chalk.cyan( | ||
| rootFilename, | ||
| )}. Skipping root route instrumentation.`, | ||
| ); |
There was a problem hiding this comment.
m: Same as above, I think we could ask users if they want to try rerunning reveal and if that still didn't work then print out what they have to add to the root file.
src/react-router/sdk-setup.ts
Outdated
| clack.log.warn( | ||
| `Failed to read ${chalk.cyan( | ||
| serverFile, | ||
| )}. Checking next server file...`, |
There was a problem hiding this comment.
m: I think we shouldn't surface this to the user, it's not actionable.
src/react-router/sdk-setup.ts
Outdated
| clack.log.warn( | ||
| `Failed to automatically update ${chalk.cyan(serverFile)}.`, | ||
| ); | ||
| // Continue to next file instead of returning false immediately |
There was a problem hiding this comment.
m: I think we should change this logic to not continue.
We found a file so it's likely the actual file users are using and continuing on doesn't seem fruitful.
We should bale out of the loop and show a pasteable snippet instead for the error case.
src/react-router/sdk-setup.ts
Outdated
| clack.log.warn( | ||
| `Could not find ${chalk.cyan( | ||
| serverEntryFilename, | ||
| )}. Skipping server entry instrumentation.`, | ||
| ); |
There was a problem hiding this comment.
m: As above, we could give users the option to reveal again and if that fails show a pasteable snippet.
src/react-router/sdk-setup.ts
Outdated
| await showCopyPasteInstructions({ | ||
| filename: serverEntryFilename, | ||
| codeSnippet: sentryServerCode, | ||
| hint: 'Add this error handling to your server entry file', | ||
| }); |
There was a problem hiding this comment.
m: Let's move this to the outer catch and rethrow here.
src/react-router/templates.ts
Outdated
|
|
||
| if (enableTracing) { | ||
| integrations.push( | ||
| 'browserTracingIntegration({\n useEffect,\n useLocation,\n useNavigate\n })', |
There was a problem hiding this comment.
m: I think we're supposed to use the reactRouterTracingIntegration instead.
There was a problem hiding this comment.
@onurtemizkan the integration also doesn't need the hooks to be passed in
| }); | ||
| } catch (e) { | ||
| clack.log.warn( | ||
| `Could not configure Vite plugin for sourcemap uploads. Please configure it manually using instructions from https://docs.sentry.io/platforms/javascript/guides/react-router/sourcemaps/`, |
There was a problem hiding this comment.
| `Could not configure Vite plugin for sourcemap uploads. Please configure it manually using instructions from https://docs.sentry.io/platforms/javascript/guides/react-router/sourcemaps/`, | |
| `Could not configure Vite plugin for sourcemap uploads. Please configure it manually using instructions from https://docs.sentry.io/platforms/javascript/guides/react-router/#source-maps-upload`, |
| return false; | ||
| } | ||
|
|
||
| export async function instrumentSentryOnEntryServer( |
There was a problem hiding this comment.
h: We also need to update the default export here (handleRequest), otherwise there will be no distributed tracing
…ntial `handleError` export types
Added
react-routerwizard, similar to ourremixwizard