Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change warning of bundling/merging if request protocol is HTTP/2 #4

Closed
JeroenVanLeusden opened this issue Oct 19, 2017 · 3 comments
Closed

Comments

@JeroenVanLeusden
Copy link

We should check if request protocol is HTTP/2 and if so, set the status of merging and bundling css/js to "OK" even though they're disabled.

For more information click here.

@jeroenvermeulen
Copy link
Contributor

@JeroenVanLeusden Thank you, you are right. I will change it.
In Magento 2.2 the config setting has disappeared in production mode so there we don't show the check.

@hostep
Copy link

hostep commented Oct 22, 2017

Please try to make this decision based on actual proof, not on some theoretical basis. It would be great if someone could actually benchmark to see if this is true or not.
I think it might be true for css files, but not for javascript files in Magento2.

I personally haven't seen any super big performance gains when using HTTP/2 because all the small js files are loaded through requirejs when not bundled/merged and that doesn't seem to benefit from HTTP/2 a lot.
But that was almost a year ago, maybe things have changed in the mean time ...

It might get better once this is integrated in Magento2 I think: https://github.com/antonkril/magento-rjs-config

Please don't assume that what I tell here is true, nothing of the above is based on scientific evidence :)

jeroenvermeulen pushed a commit that referenced this issue Dec 30, 2017
  * Added check if web server is running HTTP/2
  * Disabled checks for JS/CSS Bundling/Merging if on HTTP/2
  * Fixes issue #4. Thanks to @JeroenVanLeusden for reporting.
@jeroenvermeulen
Copy link
Contributor

Fixed in v1.13.0

jeroenvermeulen pushed a commit that referenced this issue Dec 30, 2017
  * Added check if web server is running HTTP/2
  * Disabled checks for JS/CSS Bundling/Merging if on HTTP/2
  * Fixes issue #4. Thanks to @JeroenVanLeusden for reporting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants