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): Make lazy-loading configurable #7232
Conversation
) { | ||
var lazy = true; | ||
var lazy = _lazy; | ||
var forceLoad = false; | ||
|
||
for (var i = 0; i < document.scripts.length; i++) { |
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.
Just to be clear, we still want this to be overwritable by the script tag here:
lazy = !(document.scripts[i].getAttribute('data-lazy') === 'no'); |
Or should we guard that to be basically:
var lazy = _lazy;
// if lazy is set to false, we cannot opt in?
if (lazy) {
// check if option out of lazy
for (...) {}
}
Just thinking, because I guess it would kind of break in the case of performance/replay if this were to be overwritten to true
again?
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.
Just to be clear, we still want this to be overwritable by the script tag here:
Yup, the script tag always supercedes this. Here are the scenarios:
- lazy = true + data-lazy not set -> lazy == true (default for errors)
- lazy = false + data-lazy not set -> lazy == false (default for replay + perf)
- lazy = true + data-lazy === 'no' -> lazy == false
- lazy = false + data-lazy === 'no -> lazy == false
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.
- lazy = false + data-lazy not set -> lazy == false (default for replay + perf)
but wouldn't this case, with the current code, lead to lazy == true
? 🤔
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.
you're right 🤦 - I misunderstood the boolean condition ahhhhh
Let me change this
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.
Fixed with f09a965!
size-limit report 📦
|
ref getsentry/sentry#44225
As per the work in getsentry/sentry#44876, make lazy loading a injectable option.
See https://docs.sentry.io/platforms/javascript/install/lazy-load-sentry/#load-timing for more details on how this works.