Loadable-components: Consider using the defer attribute instead of async for script tags [performance]

[charlespwd]

🚀 Feature Proposal

It seems like the most performant way of loading scripts is to include them with defer instead of async. At least according to this article

We'd be getting the following:

Instead of:

And when it comes to SSR, it would mean the js would only execute _in order_ once the HTML is fully loaded.

[charlespwd]

However, that would delay the DCL event. Thoughts?

[theKashey]

Defer also maintain the right execution order for scripts, and scripts has to be executed __after_ DCL - so it's always a right way to go.

[gregberge]

Scripts are not in the head, they are in the body. So I think async is better. In fact I inspired from next.js for that one. I don't think defer is better if we put it at the end of body.

[theKashey]

@neoziro - it's not about script position, only about marking them as defer, _defering_ execution to the "body end".

[gregberge]

@theKashey I understand, but I don't know if the defer attribute is useful when the script is placed at the end of the body. In the article, they talk about a script in the head of the page.

[theKashey]

It would help to maintain the script execution order, which may matter (load the main/run chunk first)

[gregberge]

I don’t want to maintain the script execution order, this is the point!

[theKashey]

With async - script execution order is random
Without async - waterfall, without any "prefetching"
With defer - waterfall __with__ prefetching.

All scripts would be loaded simultaneously, and executed only when script before got executed. It's sounds a bit safer.

[gregberge]

The webpack plugin handles it with a jsonp function. So it is totally safe today, it is more performant because the code is evaluated when loaded. With defer it is not the case. So async is the expected behaviour.

[theKashey]

I tend to disagree. Just today we went to a little trouble, where order-dependent CSS got broken due to reverted order of chunk execution.
I am not 100% how it might be possible, while according to the module system module2 should be executed module1 (module1 depends on module2), but by fact, module2 got executed after.

They were in different chunks, assembled in a third chunk and so on. Execution order might be important.

[gregberge]

There is already methods to specify additional props, I propose to make a change, if async={false} is passed, then we remove the async tag. And we can also add defer. So we let the user customize that part. I prefer to stay with async by default.

[charlespwd]

For what it's worth, moving the scripts to the body with async instead of defer increased our (local) lighthouse score by 1.

[gregberge]

@charlespwd +1 is good or not good?

[charlespwd]

@neoziro, it's good in favor of async loading.

[gregberge]

OK so I prefer to keep it like that for now. CSS is an edge case that could be addressed in other ways. Also assets loading order has been fixed in #266. It should work better now. Wait and see.

[theKashey]

So - I am pretty sure async is a __wrong__ decision.

• Defer - run scripts, in the right order, __after__ page is fully loaded
• Async - run a script once it is ready. __before__ or __after__ page is fully loaded
• React.hydrate - merge with a __existing__ HTML

The __only__ variant, when react can hydrate __is defer__. In async mode DOM three could be not complete, by any reason, and the result would be just __broken__.
You might wait for DOM-ready, but it would be equal to defer, but less semantic and safe.

[gregberge]

Let see what's going on on next : https://github.com/zeit/next.js/issues/6602

[gerardmrk]

Can confirm async attribute messes up css styles on first page load. Using defer with webpack's ext-html-plugin resolves it, but i rather dynamically fetch script tags on server instead of injecting them at build time.

[yairkukielka]

Defer isn't really more performant, as @charlespwd said. But it's necessary to guarantee order of script execution.

In some of my tests, using async worked on Chrome because it was trying to be 'smart' and guessing dependencies between scripts, but this was failing in other browsers.

If you have 2 scripts and one depends (or should be executed after) the other script you should use defer, as @theKashey said.

I see here that async is hardcoded. I would suggest letting the developer choose (through options) which script attributes they want to use.

[gerardmrk]

I'm doing this for now as a temp workaround till they fix it (doesn't cover any edge cases):

private _replaceScriptAsyncToDefer = async (scriptTags: string) => {
  if (!scriptTags) {
    return Promise.resolve("");
  }

  return Promise.resolve(scriptTags.replace(/\sasync\s/g, " defer "));
};

https://github.com/gerardmrk/erogen/blob/master/app/client.web/src/renderer/index.tsx#L220

[gregberge]

Yeah we should give the possibility to the user to choose between the two, I created an issue #337.

[yairkukielka]

Thanks for opening that issue, @neoziro !

I was thinking of overriding the default behavior passing a function to chunkExtractor.getScriptElements(). According to the documentation you can pass a function to it, but reading the source code it looks like this wouldn't work (because it only expects an object). Should the docs be updated or should this behavior be implemented?

[gregberge]

@yairkukielka the documentation and the code are up to date, see https://github.com/smooth-code/loadable-components/blob/master/packages/server/src/ChunkExtractor.js#L26

[yairkukielka]

I missed this! Thanks for pointing it out! :)

[seeliang]

to find out the details, I did this
https://github.com/seeliang/client-side-performance/blob/master/load-fallback/index.html

hope it could help others

[shivang2joshi]

So it turns out finally it has an optional argument support there.

exractor.getScriptTags({ defer: "", anyAttrToScript: "youGotIt" });

This is reference to everyone who browses this issue should get the answer at last (this) comment. 😃 Please add this use case scenario in docs as well. @gregberge in Docs, it is not clear that exactly how the arguments should be passed and so eventually one has to check in the code how is it receiving the args. So, it will be easy and time saving for everyone if an example is provided there 🙏🏻 !

[markwoodward23]

@shivang2joshi - I tried that but it purely adds to the scripts, rather than removing the async word. That's definitely hardcoded in: https://github.com/gregberge/loadable-components/blob/master/packages/server/src/ChunkExtractor.js#L46

In our case - we're loading script tags into the head, rather than the body, and want them to be defer. Our solution to this is to use the getScriptElements function on the extractor, then run them through a custom function to build the script tags with defer instead of async.

  const buildScriptTags = arr =>
    arr
      .map(({ props }) => {
        if (props.id === '__LOADABLE_REQUIRED_CHUNKS__') {
          return `<script id="__LOADABLE_REQUIRED_CHUNKS__" type="application/json">${
            props.dangerouslySetInnerHTML.__html
          }</script>`;
        } else {
          return `<script ${Object.entries(props)
            .map(([key, val]) => {
              return key === 'async' ? 'defer' : `${key}="${val}"`;
            })
            .join(' ')}></script>`;
        }
      })
      .join('\n');

  const scripts = buildScriptTags(extractor.getScriptElements());

Not the best, but it works 🤷‍♂️

[shivang2joshi]

@shivang2joshi - I tried that but it purely adds to the scripts, rather than removing the async word. That's definitely hardcoded in: https://github.com/gregberge/loadable-components/blob/master/packages/server/src/ChunkExtractor.js#L46

In our case - we're loading script tags into the head, rather than the body, and want them to be defer. Our solution to this is to use the getScriptElements function on the extractor, then run them through a custom function to build the script tags with defer instead of async.

  const buildScriptTags = arr =>

    arr

      .map(({ props }) => {

        if (props.id === '__LOADABLE_REQUIRED_CHUNKS__') {

          return `<script id="__LOADABLE_REQUIRED_CHUNKS__" type="application/json">${

            props.dangerouslySetInnerHTML.__html

          }</script>`;

        } else {

          return `<script ${Object.entries(props)

            .map(([key, val]) => {

              return key === 'async' ? 'defer' : `${key}="${val}"`;

            })

            .join(' ')}></script>`;

        }

      })

      .join('\n');



  const scripts = buildScriptTags(extractor.getScriptElements());

Not the best, but it works 🤷‍♂️

For your use case, this might work just fine. So simple, just replace async with defer in final output.