defer loading of kit loader scripts

[mlwilkerson]

closes #126

Other scripts being enqueued by wp_enqueue_script are already getting the defer attribute by default. For example, the <script> that gets added to the page when in "Use CDN" mode (versus "Use a Kit" mode) has defer, and this is not configurable.

So it seems out of place that we aren't already adding defer to the <script> in the "Use a Kit" mode. Thus, rather than making this optional or configurable, as suggested by the request in #126, it seems preferable to keep it simple, thinking of this more like a bug fix--achieving parity with the other <script> loading.

If it seems like a problem, we could add a filter. However we do that, then we might need to add different filters for different scripts since they currently already differ in the application of their defer attributes. In other words, since they don't all have the same values now, it may not work to have a single filter that sets them all to on or off.

[mlwilkerson]

One reason a reviewer might push back on this PR: it only partially addresses the concern about allowing a developer to control "rendering blocking" behavior across all <link> and/or <script> elements that this plugin might add to the page. (But hear me out: I think it might be still be adequate. Read on.)

See this related WordPress plugin forum topic. In that, the original poster user is concerned with avoiding render blocking for:

• css/all.css
• css/v4-shims.css

Those would only be seen as render blocking when the plugin is in "Use CDN" mode, because in that mode, the plugin just adds corresponding <link> tags to the page. To get those added asynchronously would require more magic. It seems like we probably should not put effort into making that magic because Font Awesome 6 Pro is not available via the legacy CDN anyway. "Use a Kit" is now the recommended way to go for either Free or Pro users.

On that linked support forum topic, I suggested switching to a Kit in order to avoid render blocking CSS for a webfont/css kit. That would have done the job for the CSS if the change proposed by this PR had already been present, because the kit loader script does fetch injection to add the CSS as <style> elements asynchronously. However, the kit loader <script> itself was not being loaded with defer. So while it would load everything else asynchronously, itself was render blocking. That's the concern mentioned in #126.

So another way to look this: if the change proposed by this PR had already been completed, then my recommendation in that linked support topic would have worked: simply switching to a webfont/css kit would have eliminated all render blocking JavaScript and/or CSS.

[alexpoiry]

The issue, as I see it, is that we (or at least I) don't actually know why we're doing this. Based on what I've read in 126, some number of customers (more than one) have asked for a specific solution base on maybe a performance auditing tool's results. As is often the case with auditing tools, they can fixate on things that aren't a significant problem, i.e., loading a very small script without defer or async (Rob mentioned this). For all I know, this is based of the request of some fresh-faced college kids who haven't yet learned that blindly accepting the results of static analysis is generally not a good idea.

My consultant brain says we have a solution without a fully defined problem.

That being said, making it configurable seems like overkill in any case (especially without a fully documented use case). To apply due diligence (or due-er diligence since we should really validate that we truly understand the problem {but I assume that ship has sailed as this request is 2 years old}) we have to ask the question, what are the risks of making the script defer by default?

Based on the previous discussions, it appears that the script loads all other items as defered anyway and the CDN was using this all along. So, unless there is something special about the kits that is likely to break something if that initial script doesn't block, I think this solution is fine.

ALL that being said, due-est diligence would include doing something to prove or at least suggest that this change won't break things under common conditions at least (and under any condition at most but I find it unlikely that we'd be able to enumerate all those possibilities).

So... How confident are you that this change won't break something else? Would you, for example, let me punch you really hard in the arm if you're wrong? (Something I would not do anyway because the risks generally outweigh the rewards, but you get the point.)

[mlwilkerson]

I do suspect that the problem is mainly: people want to see a clean analysis tool report, and those tools have a heuristic of penalizing render blocking scripts. So this change probably does not make a significant improvement to the actual performance and functionality of page (though I have not proven that). It may only serve to "lint" a performance report. To the extent that framing is accurate, these analysis tools produce a false perception of a performance problem; this change removes that false perception.

I strongly believe that there is no new problem that is introduced by this change, enough that you could punch me in the arm if I'm wrong 🦾 . I moderately believe that this change's only value is to eliminate a false perception of a performance problem due to being slightly, technically, briefly render-blocking. So I think it's an acceptable trade-off. If this change improves perception without any significant change one way or another, then we might as well improve perception.

The other side of this, is that, I think we already accepted this compromise long ago, as evidenced by the fact that the past me had already hardcoded defer on the other scripts loaded by this plugin. So I think it's likely that the subsequent past me--when adding support for kits--simply overlooked that fact.