marko-js/writable-dom

should keep script async attribute to false for block executing scripts

kuitos opened this issue · 6 comments

Version: 1.0.3

Details

I found that non-async scripts marked with defer will become async after insertion, which causes the defer scripts not to be executed in document order. There is no problem with non-defer synchronous scripts because we have a blocking flow that waits for the previous script to finish before loading the next one.

I did some digging and found that this is because scripts written through a documentFragment will all become async true, and the same issue exists with document.importNode.
Here is example code to explain the issue:

const doc = document.implementation.createHTMLDocument('');
doc.write('<!DOCTYPE html><body><template>');
const root = (doc.body.firstChild).content;
const walker = doc.createTreeWalker(root);
doc.write('<script src="./a.js"></script>');

console.log(walker.root.children[0].async);  // true
const script = document.createElement('script');
script.async = false;
const clone = document.importNode(script);
console.log(clone.async); // true

Expected Behavior

defer script without async attribute on html should execute in the order in which they appear in the document.

Actual Behavior

defer script without async attribute on html not executed in order

Possible Fix

set script element async attribute correctly before it been append to target document

const parentNode = targetNodes.get(node.parentNode!)!;

if (isSyncScript(clone)) {
  clone.async = false;
}

Your Environment

  • Chrome 119
  • MacOS 14.0

@kuitos curious what impact you are seeing from this behavior? Currently the blocking scripts are handled by writabledom itself without changing the underlying property (just uses attribute presence). Changing the property wouldn't really have an impact besides other code explicitly reading that async property?

(specifically here it uses an attribute check instead of a property check https://github.com/marko-js/writable-dom/blob/main/src/index.ts#L161)

Oh my bad your issue is with "defer" scripts.
Yeah that isn't currently well supported, but probably should be.

FWIW I recommend avoiding defer scripts and instead preferring actual asyn scripts (especially when streaming)

Yeah this issue is all about defer scripts.

FWIW I recommend avoiding defer scripts and instead preferring actual asyn scripts

But in my knowledge, some popular frontend frameworks used defer to improve there first screen performance by default, especially in streaming ssr mode, like vue and nextjs.

The problem with defer is that these scripts cannot execute until the entire page is done, but with async you have (in a non blocking way) the script execute as soon as possible.

if the framework you're using supports async hydration then you should always use async over defer since the user will be able to interact with the page sooner.

Either way ideally this module would support defer.

It's a bit confusing as to way defer should do in a micro-frame though. Normally defer waits until DOMContentLoaded to execute, however I guess here it'd have to wait just until the writable dom instance has closed. One problematic piece here is that if you use micro-frame or another wrapper of this module which attempts to make it isomorphic there is no way I can think of to actually make the defer scripts operate that when when server rendering. Not that this is necessarily a big deal for this module.

Normally defer waits until DOMContentLoaded to execute, however I guess here it'd have to wait just until the writable dom instance has closed.

Yes, that's what I was thinking. Another problem is how to block the execution of deferred scripts. My thought is to remove their src attribute and re-add it until the writable stream is closed.

One problematic piece here is that if you use micro-frame or another wrapper of this module which attempts to make it isomorphic there is no way I can think of to actually make the defer scripts operate that when when server rendering.

Sorry I am not familiar with micro-frame, I'm not sure is defer script will be used when server-side rendering? As far as I know, the defer script string is written to the HTML response during server rendering and then executed by the browser client, the server does not interact with the defer script in HTML stream, is my understanding correct?

If we don't consider fully consistent to browser behavior for now (defer scripts must execute before DOMContentLoaded but after HTML parsed), and only need to ensure that defer scripts execute in order, as long as they are inserted into the document with async set to false before execution, does this make sense?