-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: fatal error when current JS is not inside script tag #2229
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
* for example when html-webpack-inline-source-plugin is used, the JS is inlined in HTML in script tag. So in this case document.currentScript.getAttribute('src') is undefined * undefined cannot be used as string with method replace. This cause a fatal error and crash app
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.
Need tests
Hi, I'm sorry for failing PR, but I added a special case to tests, where getCurrentScriptSource returns undefined. So it could be enough for test coverage. It's exactly the value, which would have been failing tests when my change is not applied. |
We need fix CI error |
Codecov Report
@@ Coverage Diff @@
## master #2229 +/- ##
=========================================
+ Coverage 93.89% 93.9% +<.01%
=========================================
Files 34 34
Lines 1278 1279 +1
Branches 366 367 +1
=========================================
+ Hits 1200 1201 +1
Misses 71 71
Partials 7 7
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #2229 +/- ##
=========================================
+ Coverage 93.89% 93.9% +<.01%
=========================================
Files 34 34
Lines 1278 1279 +1
Branches 366 367 +1
=========================================
+ Hits 1200 1201 +1
Misses 71 71
Partials 7 7
Continue to review full report at Codecov.
|
Closing in favor of #2246 |
For Bugs and Features; did you add new tests?
Motivation / Use-Case
Breaking Changes
Additional Info