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

feat(loader): Sync loader with JS monorepo #43880

Merged
merged 2 commits into from
Feb 1, 2023
Merged

Conversation

AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Jan 31, 2023

As per getsentry/sentry-javascript#6990 the loader in the JS monorepo and sentry template has drifted apart. This syncs them together (validated by the integration tests in the JS monorepo).

PR in JS Repo: getsentry/sentry-javascript#7001

(content.e ||
content.p ||
('e' in content ||
'p' in content ||
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -56,7 +56,7 @@
// come out in the wrong order. Because of that we don't need async=1 as GA does.
// it was probably(?) a legacy behavior that they left to not modify few years old snippet
// https://www.html5rocks.com/en/tutorials/speed/script-loading/
var _currentScriptTag = _document.getElementsByTagName(_script)[0];
var _currentScriptTag = _document.scripts[0];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -68,6 +68,9 @@
_window[_onerror] = _oldOnerror;
_window[_onunhandledrejection] = _oldOnunhandledrejection;

// Add loader as SDK source
_window.SENTRY_SDK_SOURCE = 'loader';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tracekitErrorHandler.apply(_window, data[i].e);
} else if (data[i].p && tracekitUnhandledRejectionHandler) {
} else if ('p' in data[i] && tracekitUnhandledRejectionHandler) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

queue({
p: exception.reason
p: 'reason' in e ? e.reason : 'detail' in e && 'reason' in e.detail ? e.detail.reason : e
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kamilogorek
Copy link
Contributor

LGTM!

@AbhiPrasad AbhiPrasad merged commit 9b813e6 into master Feb 1, 2023
@AbhiPrasad AbhiPrasad deleted the abhi-sync-js-loader branch February 1, 2023 10:50
@github-actions github-actions bot locked and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants