-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(js-loader): Only use es5/es6 bundles in v7 JS loader #66303
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #66303 +/- ##
========================================
Coverage 84.29% 84.29%
========================================
Files 5306 5309 +3
Lines 237041 237228 +187
Branches 41020 41039 +19
========================================
+ Hits 199805 199982 +177
- Misses 37017 37027 +10
Partials 219 219
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to add some tests to the existing loader tests for this change.
We can probably copy & paste the implementation from here:
sentry/tests/sentry/web/frontend/test_js_sdk_loader.py
Lines 85 to 100 in 6cb54b1
@mock.patch( | |
"sentry.loader.browsersdkversion.load_version_from_file", return_value=["6.19.7", "7.0.0"] | |
) | |
@mock.patch( | |
"sentry.loader.browsersdkversion.get_selected_browser_sdk_version", return_value="7.x" | |
) | |
def test_equal_to_v7_returns_es5( | |
self, load_version_from_file, get_selected_browser_sdk_version | |
): | |
settings.JS_SDK_LOADER_DEFAULT_SDK_URL = "https://browser.sentry-cdn.com/%s/bundle%s.min.js" | |
self.projectkey.data = {} | |
self.projectkey.save() | |
resp = self.client.get(self.path) | |
assert resp.status_code == 200 | |
self.assertTemplateUsed(resp, "sentry/js-sdk-loader.js.tmpl") | |
assert b"/7.0.0/bundle.es5.min.js" in resp.content |
@@ -67,7 +67,8 @@ def _get_loader_config( | |||
"hasDebug": False, | |||
} | |||
|
|||
is_greater_or_equal_v7_sdk = sdk_version >= Version("7.0.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can leave the name of is_greater_or_equal_v7_sdk
as-is. Imo it doesn't add any value renaming and that way git blame is a bit more useful.
As the coming version of the JS SDK only uses ES2017 bundles, it is no longer necessary to differentiate between es5 and es6 bundles.
ref getsentry/sentry-javascript#10911