Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set root for RewriteFrames integration #650

Merged
merged 1 commit into from
Nov 19, 2023
Merged

Conversation

rchl
Copy link
Member

@rchl rchl commented Nov 18, 2023

After manually inspecting some crashes I've realized that the RewriteFrames integration that the module enables by default does more harm than good.

In case there are source maps it doesn't really make any difference since stack frame paths are resolved from source maps then.

In case there are no source maps (for example when crash is within the server code and not inside the webpack context) then what RewriteFrames does is it strips the whole path leaving just the file name. That can result in just index.js being shown, for example, which makes it unclear which file is being referred to.

To fix that, set root option on the server so that the paths are resolved relative to Nuxt's rootDir.

Also don't enable RewriteFrames integration on the client since it doesn't seem to be useful there. It only does something when the stack frame path is a local file path and on the client-side I don't see this being the case.

Copy link

size-limit report 📦

Path Size
fixture: base 379.64 KB (-0.57% 🔽)
fixture: replay 498.79 KB (-0.43% 🔽)
fixture: lazy 384.43 KB (-0.56% 🔽)
fixture: tracing 399.83 KB (-0.54% 🔽)
fixture: lazy+tracing 404.59 KB (-0.53% 🔽)
fixture: typescript 379.77 KB (-0.57% 🔽)

@rchl rchl changed the title fix: set root for RewriteFrames integration disable on the client fix: set root for RewriteFrames integration Nov 19, 2023
@rchl rchl merged commit d8c4733 into main Nov 19, 2023
6 checks passed
@rchl rchl deleted the fix/rewrite-frames branch November 19, 2023 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant