magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.45k stars 9.29k forks source link

What is the purpose of the defer attribute in the linkType elements (css, link, font)? #37657

Open nurullah opened 1 year ago

nurullah commented 1 year ago

Summary

While I searching for ways to defer non-critical CSS files, I found that magento2 has a defer attribute in the head.xsd file. This means I can write the following code in the default_head_blocks.xml without any errors.

<css src="css/intl-tel-input.css" defer="true" />

But I realized that this defer attribute unfortunately has no effect on the CSS element. I just wondering the purpose of allowing us to use this attribute. Do you have any future plans about this attribute for any of the linkType elements (css, link, font)?

Examples

https://github.com/magento/magento2/blob/2.4-develop/lib/internal/Magento/Framework/View/Layout/etc/head.xsd#L11C27-L11C27

Proposed solution

<css src="css/intl-tel-input.css" defer="true" />

After editing the Renderer.php file, we can get the output as follows by writing the code above. With this way, we can defer non-critical render-blocking CSS files on the first load.

<link rel="preload" href="css/intl-tel-input.css" as="style" onload="this.onload=null;this.rel='stylesheet'">
<noscript><link rel="stylesheet" href="css/intl-tel-input.css"></noscript>

Release note

No response

Triage and priority

m2-assistant[bot] commented 1 year ago

Hi @nurullah. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

Usik2203 commented 1 year ago

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 1 year ago

Hi @Usik2203. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 1 year ago

Hi @Usik2203, here is your Magento Instance: https://8464f07e498f3fc9d0b2db82f15bdb09.instances-prod.magento-community.engineering Admin access: https://8464f07e498f3fc9d0b2db82f15bdb09.instances-prod.magento-community.engineering/admin_9e1a Login: 802c155e Password: a2220159c4ba

mrtuvn commented 1 year ago

From my experience defer only work with script tag ! I'm not to try with css. So i think you may to want use critical css right ? Not sure it's fit with your case or you just want specific priority load for some speicifc css file

nurullah commented 1 year ago

From my experience defer only work with script tag ! I'm not to try with css. So i think you may to want use critical css right ? Not sure it's fit with your case or you just want specific priority load for some speicifc css file

It's easier to know which CSS files to defer than to know which CSS codes are absolutely necessary. I used critical CSS but it causes CLS issues when not prepared perfectly.

mrtuvn commented 1 year ago

so from my understanding we need somehow to specific defer to css link When we add defer attribute css will render output -> <link rel="stylesheet" media="all" type="text/css" href="css/intl-tel-input.css" as="style" onload="this.onload=null;this.media='all';this.rel='stylesheet'">

without specific link css will output as normal

nurullah commented 1 year ago

so from my understanding we need somehow to specific defer to css link When we add defer attribute css will render output -> <link rel="stylesheet" media="all" type="text/css" href="css/intl-tel-input.css" as="style" onload="this.onload=null;this.media='all';this.rel='stylesheet'">

without specific link css will output as normal

Rel attribute must have preload value in your example. I was trying to get that output with the current head elements but I can't add onload attribute. Because it is not supported in <css .. /> and <link .. /> elements. So, I added the below XML code in the default_head_blocks.xml file.

<css src="css/intl-tel-input.css" rel="preload" as="style" />

But I got the below output: Preload attribute was defined twice, i couldn't pass the onload attribute and i couldn't add the <noscript>...</noscript> definition that will work if JS turned off in the browser.

image

mrtuvn commented 1 year ago

After more dig in code base i see team already have some work on this You can see this line https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Theme/Controller/Result/AsyncCssPlugin.php#L104 Note sure this is complete cover all edge-cases

mrtuvn commented 1 year ago

I don't see any check defer property anywhere else in php code -> for css type

mrtuvn commented 1 year ago

in my opinion this one should consider as bug -> defer attribute not correct render for css link Scenario 1: No enable critical css -> If no specific link defer -> Work as current. If link css defer specific then reformat link Scenario 2: Enable critical css -> critical asset will go first -> Defer specific link will reformat as nurullah's suggest. Other links non-defer will render as normal

nurullah commented 1 year ago

in my opinion this one should consider as bug -> defer attribute not correct render for css link Scenario 1: No enable critical css -> If no specific link defer -> Work as current. If link css defer specific then reformat link Scenario 2: Enable critical css -> critical asset will go first -> Defer specific link will reformat as nurullah's suggest. Other links non-defer will render as normal

I worked Scenario 1 method in my local environment but it's not ready for the pull request. When I find the some spare time, I will send pull request for this issue.

nurullah commented 1 year ago

@magento I am working on this.

m2-assistant[bot] commented 1 year ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Hotel commented 1 year ago

Hello @nurullah,

Thanks for the report and collaboration!

@mrtuvn thanks for your inputs on this issue.

We have tried to reproduce the issue via adding defer=true attribute to calendar.css as well as tried to add rel="preload" as="style" but it does seems to work as expected. Please find the below screenshot for reference:

While adding defer=true

image

It still shows the priority as Highest: image

While adding rel="preload" as="style"

It is adding rel attribute 2 times as below:

image

Hence confirming the issue.

Thanks

github-jira-sync-bot commented 1 year ago

:x: Something went wrong. Cannot create Jira issue.

nurullah commented 1 year ago

Hello @nurullah,

Thanks for the report and collaboration!

@mrtuvn thanks for your inputs on this issue.

We have tried to reproduce the issue via adding defer=true attribute to calendar.css as well as tried to add rel="preload" as="style" but it does seems to work as expected. Please find the below screenshot for reference:

While adding defer=true

image

It still shows the priority as Highest: image

While adding rel="preload" as="style"

It is adding rel attribute 2 times as below:

image

Hence confirming the issue.

Thanks

Hello.

HTML link element doesn't have defer attribute. We need to use that hacky method in the image below, when we want to defer CSS files.

In this example, rel attribute must be defined only one time. Because rel="preload" attribute tells to the browser load this file but don't render until the onload function run. The onload function changes the rel attribute to the stylesheet and this CSS file starts rendering after the whole document is loaded.

image