fix: fix inline scripts and styles potentially breaking #2
Conversation
|
|
|
Thanks for taking the time to put up the PR! I think it looks pretty good 👍 |
dario-piotrowicz
force-pushed
the
main
branch
2 times, most recently
from
70374ab to
9d58c8f
Compare
Codecov Report
@@ Coverage Diff @@
## main #2 +/- ##
==========================================
+ Coverage 90.24% 91.48% +1.24%
==========================================
Files 1 1
Lines 82 94 +12
Branches 26 29 +3
==========================================
+ Hits 74 86 +12
Misses 6 6
Partials 2 2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
dario-piotrowicz
force-pushed
the
main
branch
from
9d58c8f to
b0751d8
Compare
dario-piotrowicz
force-pushed
the
main
branch
from
b0751d8 to
9251b2b
Compare
dario-piotrowicz
requested review from
IgorMinar and
DylanPiercey
and removed request for
DylanPiercey and
IgorMinar
src/index.ts
Outdated
|
|
||
| function isInlineHost(node: Node) { | ||
| if (node.nodeType !== Node.ELEMENT_NODE) { | ||
| return false; |
There was a problem hiding this comment.
this line is not being covered by tests, and by checking the MDN nodeType docs I think that the node here can't be anything else but an Element, so I am just going to remove this check, please let me know if you disagree
fix the issue which breaks scripts split in different chunks in the input stream by applying them only after the scripts has been fully visited also fix the same issue for embeded styles, even though there everything works fine it is better to apply all the styles together and avoid potential flashes of unexpected style combinations resolves marko-js#1
dario-piotrowicz
force-pushed
the
main
branch
from
9251b2b to
442cfa3
Compare
|
@DylanPiercey I've added a new very simple test because it looked like my change was causing some tests coverage failure (even though the non-covered lines seem unrelated to my change 😕), I hope it's fine, if you could approve the workflow we can see if now all is green 😁🤞 |
|
Appreciate the changes and additional tests. Looks good to me! |
|
🎉 This PR is included in version 1.0.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Awesome! thanks! 😄 |
fix the issue which breaks scripts split in different chunks in the input stream
by applying them only after the scripts has been fully visited
also fix the same issue for embeded styles, even though there everything works
fine it is better to apply all the styles together and avoid potential flashes of unexpected style combinations
resolves #1
Screenshots (if appropriate):
This is the result:
Checklist: