marko-js/writable-dom

Inline scripts can get broken

dario-piotrowicz opened this issue ยท 5 comments

Version: 1.0.1

Details

Hi @DylanPiercey, awesome small library you've created ๐Ÿ˜„

Anyways one thing I noticed as I was playing with it is that inline scripts don't seem to be fully supported, what I mean is that if an inline script gets truncated in the stream, then it won't be correctly handled by the library, as the library will either execute only part of the script and/or will present a syntax error depending on where the script got truncated.

For example I have created the following Cloudflare worker which exposes a stream which truncates a very simple inline script:

addEventListener("fetch", event => {
  event.respondWith(handleRequest(event.request))
})

async function handleRequest(request) {
  return new Response(getStream()});
}

function getStream() {
  const { writable, readable } = new TransformStream();
  writeInStream(writable);
  return readable;
}

const chunks = [
`<script>
  console.log('`,`this is streaming!');
</script>`,
];

async function writeInStream(writable) {
  const encoder = new TextEncoder();
  const writer = writable.getWriter();
  const write = str => writer.write(encoder.encode(str));

  for (const chunk of chunks) {
    write(chunk);
    await new Promise(resolve => setTimeout(resolve, 1000));
  }

  writer.close();
}

(well, the implementation there is not important, the relevant thing is that the stream is comprised of two separate chunks:
<script>console.log(' and this is streaming!');</script>)

If I run the code you present in the Repo's readme (in which of course I replace "https://ebay.com" with the worker's URL)
I get a syntax error in the console:
Screenshot 2022-09-14 at 22 44 51

The issue being that initially the script gets only partially added to the document:
Kapture 2022-09-14 at 22 48 38

I think that the script's content should be buffered and that the script should be added fully to the document in a single operation, so that the whole script will be evaluated correctly by the browser.

Naturally my example here is a silly one, but I came across this issue as I was trying to use the library and the stream I was trying to add to the dom happens to get truncated causing a syntax error and making the whole thing not work.

Have you considered this issue? and/or do you have an opinion/potential solution for it?

As I'm interested in what this library is doing, I wouldn't mind trying to add a fix and submit a PR ๐Ÿ™‚

@dario-piotrowicz ah this is a good catch. I don't have time to dig into this right away but would happily review a PR.
Probably the logic needs to be updated to avoid setting text content for <script> and probably <style> until they are complete as you say.

Ah ok thanks, I'll see what I can do then! ๐Ÿ˜ƒ๐Ÿ‘

By the way, this is what happens with styles:

Kapture 2022-09-15 at 11 31 22

They are streamed correctly and everything works ok (and there are no errors in the console)

The only issue being that only the partially streamed styles get applied, like in my gif for example you can see the black background being applied first and the green color being applied only on the next chunk.

What do you think? is this behavior ok? or should the styles be blocking and appear all in one go?

@dario-piotrowicz I think to be consistent with browser behavior the styles should be displayed all at once with the related tag. Otherwise there could be flashes of unexpected style combinations.

๐ŸŽ‰ This issue has been resolved in version 1.0.2 ๐ŸŽ‰

The release is available on:

Your semantic-release bot ๐Ÿ“ฆ๐Ÿš€