Skip to content

Extra gatsby-ssr.js file in npm package #13

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

Closed
browniebroke opened this issue Jan 10, 2019 · 8 comments
Closed

Extra gatsby-ssr.js file in npm package #13

browniebroke opened this issue Jan 10, 2019 · 8 comments
Assignees

Comments

@browniebroke
Copy link

I'm not sure why I'm getting this given that it was removed in #9, but I've installed the latest version (1.0.0) from NPM, and there is a extra gatsby-ssr.js in the installed version of gatsby-plugin-sentry I'm getting in my node_modules.

I don't think it's causing any problems for me, but it looks like something went wrong when deploying the latest release.

@octalmage
Copy link
Owner

Good catch! I'll get that fixed. Thanks1

@octalmage octalmage self-assigned this Jan 13, 2019
@humphreybc
Copy link

humphreybc commented Jan 15, 2019

Found this one too. I think this is causing the script tag to be added to the head and the JS to be loaded synchronously from the ravenjs.com CDN. This is causing some performance issues (indeed PageSpeed Insights doesn't like it at all) and basically makes this issue still a problem.

Would be great to get an ETA on a fix! :)

@mhadaily
Copy link
Collaborator

@humphreybc Yes, it loads synchronously and of course, we can load it asynchronously, however, there is a limitation as of now which is, Sentry will capture only global errors and unhandled promise until it will be fully loaded and available, if anything happens during download and initialization like an error in XHR request, click on something or anything similar, it will not be captured.

I think it's good to have both Sync and Async option for this plugin and it should be up to the user to choose whether they want Lazy load or synchronous load. what do you think @humphreybc and @octalmage? (BTW, I can sed a PR for this if everyone agrees)

@humphreybc
Copy link

@mhadaily Yeah, an async: true option that we could set in gatsby-config would be awesome.

@antoinerousseau
Copy link

antoinerousseau commented Jan 23, 2019

Why load Sentry both as a CDN script tag in gatsby-ssr.js AND an NPM package in gatsby-browser.js? Wouldn't the latter be enough?

@mhadaily
Copy link
Collaborator

@antoinerousseau I suppose gatsby-ssr.js is extra and will be removed, @octalmage is working on it.

@humphreybc
Copy link

It would be great to get a fix for this. Unfortunately we've had to disable this plugin for now because it's destroying our page speed score. Would love to re-enable it.

@octalmage
Copy link
Owner

The fix has been deployed as v1.0.1. Really not sure what happened there, let me know if you still encounter issues!

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

No branches or pull requests

5 participants