POC for synchronous error rendering - #179
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| }); | ||
|
|
||
| if (process.env.NODE_ENV === 'development') { | ||
| import('@spotlightjs/astro').then(Spotlight => { |
There was a problem hiding this comment.
so we could actually add localStorage settings into spotlight and make this a default behavior for any unhandled error coming in too.
can also keep this behavior (i think we NEED it) and continue w/ a setting to toggle it
| @@ -4,130 +4,102 @@ import Card from '../components/Card.astro'; | |||
| --- | |||
There was a problem hiding this comment.
just linter kicking in on this file
|
To ship this theres a few things, some imo required, some could be second pass:
|
I guess the tricky bit here, and what we'd probably want to do... |
| <App | ||
| integrations={initializedIntegrations} | ||
| fullScreen={fullScreen} | ||
| defaultEventId={defaultEventId} |
There was a problem hiding this comment.
I think we can remove this option then, right? (already marked as a removal candidate in the JSDoc)
There was a problem hiding this comment.
I think the big problem here is that we can't reliably detect what kinds of errors should open Spotlight vs. which ones shouldn't.
Checking handled won't do the trick because JS SDKs always set handled: false unless users manually called captureException.
| const onRenderError = (e: CustomEvent) => { | ||
| log('Sentry Event', e.detail.event_id); | ||
| if (e.detail.event) sentryDataCache.pushEvent(e.detail.event); | ||
| // TODO: handle async | ||
| openSpotlight(`/errors/${e.detail.eventId}`); | ||
| }; |
There was a problem hiding this comment.
wdyt about passing openSpotlight to the setup hook callback instead of importing it from ~/index?
I think this would isolate the integrations more from the skeleton.
There was a problem hiding this comment.
That could work though we likely need a larger context manager. I dont think we should abstract too much away from React
4ccdfeb to
0866119
Compare
- Bump hono direct dep from ^4.12.4 to ^4.12.7 (prototype pollution fix) - Add pnpm overrides for: flatted (>=3.4.0), yauzl@>=3 (>=3.2.1), devalue (>=5.6.4), rollup@>=4 (>=4.59.0), minimatch (all ranges), ajv (both v6 and v8 ranges) - Update existing tar override from >=7.5.4 to >=7.5.11 - Migrate Yarn resolutions block to pnpm.overrides (pnpm ignores the resolutions field entirely) - Dismiss 4 unfixable alerts: svelte (not used, optional peer dep), @tootallnate/once (tolerable risk, dev-only, locked to v2), yauzl v2 (tolerable risk, dev-only, locked by electron/extract-zip) Fixes dependabot alerts: #156, #158, #159, #160, #161, #162, #163, #164, #165, #167, #168, #169, #170, #171, #172, #173, #179, #180, #181, #182, #183, #185 Dismisses: #157, #166, #178, #184
This branch was successfully deployed
2 participants
High level:
Spotlight.trigger(eventName, eventPayload)