-
-
Notifications
You must be signed in to change notification settings - Fork 470
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: check if 'btoa' exists for old IE versions #479
fix: check if 'btoa' exists for old IE versions #479
Conversation
Codecov Report
@@ Coverage Diff @@
## master #479 +/- ##
=======================================
Coverage 98.68% 98.68%
=======================================
Files 5 5
Lines 228 228
Branches 96 96
=======================================
Hits 225 225
Misses 3 3
Continue to review full report at Codecov.
|
c0de7ca
to
4dd10b8
Compare
@@ -187,7 +187,7 @@ function applyToTag(style, options, obj) { | |||
style.removeAttribute('media'); | |||
} | |||
|
|||
if (sourceMap && btoa) { | |||
if (sourceMap && typeof btoa !== 'undefined') { |
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.
btoa
should be undefined
for old IE, can you provide screenshot?
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.
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.
typeof
should be used to check if global variables exist.
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.
What is IE version? It is weird, because btoa should be false here
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.
In IE9, btoa
is undefined
.
Or the following code is also good.
if (sourceMap && window.btoa) {
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 am fine with typeof
, weird, I can't reproduce
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 have created an example of a problematic code to reproduce the phenomenon.
<script>
if (btoa) {
console.log("it's exist");
} else {
console.log("it's not exist");
}
</script>
This code causes an error in IE9.
Please check the console window on this page.
http://okchangwon.xyz/naver/ie9-btoa
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.
error from ie9-btoa
package, not from style-loader
Let me summarize it overall. In the style loader's applyToTag method The reason for checking the existence of Very good so far. But the problem is, It is currently as follows. if (sourceMap && btoa) If you access a nonexistent global variable (or function) like this
Because there is no btoa in 'IE9', an error occurs. So I recommend the following method. if (sourceMap && window.btoa)
// or
if (sourceMap && typeof btoa !=='undefined') Please let me know if I'm misunderstanding the situation. |
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.
Let's merge
This PR contains a:
Motivation / Use-Case
If the
btoa
function does not exist, an error will occur as follows.Breaking Changes
Additional Info