Yield out potentional slot instructions when rendering dynamic tags#4981
Yield out potentional slot instructions when rendering dynamic tags#4981
Conversation
🦋 Changeset detectedLatest commit: c586942 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
f70941a to
c586942
Compare
|
!bench |
|
Node: 14 Node: 16 |
natemoo-re
left a comment
There was a problem hiding this comment.
LGTM assuming the bench deviation is not significant!
|
I really love that command. Just need to clean up the output a little bit. |
| let i = 0; | ||
| for await (const chunk of iterable) { | ||
| if (isHTMLString(chunk)) { | ||
| if (i === 0) { |
There was a problem hiding this comment.
I cant see where i is being incremented within the for of loop, it reads *forgive me, as a unnecessary conditional check, I mean its perhaps somewhere but I just cant see it from here,
aFuzzyBear
left a comment
There was a problem hiding this comment.
Hey Matt, I am sorry I know this is merged, but I was taking a wee look over this, just doing some studying, and I got stumped on a few things. predomately with the iterableToHTMLBytes now Im not pretending to understand whats going on, kinda got a wee sense, but two things struck out to me.
- The logic is very nested, having looked at it I was wondering if it could be less deep
for await (const chunk of iterable) {
- if (isHTMLString(chunk)) {
- if (i === 0) {
- if (!/<!doctype html/i.test(String(chunk))) {
+ if ( isHTMLString(chunk)
+ && i === 0
+ && !/<!doctype html/i.test(String(chunk))
) { parts.append('<!DOCTYPE html>\n', result);
if(onDocTypeInjection) {
await onDocTypeInjection(parts);
}
}
}
}Also what is i doing exactly, I couldnt see where it gets incremented,
Sorry to take your time with this,
|
Yeah we can talk about it. Are you investigating because you've encountered a bug you think was caused by this? |
|
no no, I was doing a wee bit of studying last night, if you got the time dude Id love to understand this whole thing alot better |
|
There was a bug that came about from this, fixed here: #5035 |
Changes
Testing
Docs
N/A, bug fix