-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[AssetMapper] Adding nopush to Link header #52143
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
Conversation
If nopush also disables the 103 early hints, that would be a bad move IMO. |
note that as Chrome indicates that it does not want pushed resources, servers won't push them those resources. |
The question is: for people using Firefox, Safari, are those also indicating that they don't want pushed resources? I don't know the answer. And which web servers are pushing resources? Is this a real problem? Or in practice, is nothing being pushed already?
I think you're correct that we would NOT want to include |
AssetMapper is not the one sending the response at all when you render an import map. So early hints might still be used. |
Hmm. Here is the only flow I can imagine: A) User manually adds I'm not entirely sure what happens in this case. The best I can tell, this would be ok - but it is a bit odd. However, closing this PR would be a bigger problem, assuming that server-push is actually something that's happening in practice with non-Chrome browsers. |
The As @stof pointed out, Chromium-based browsers no longer support Server Push, and although SP is still supported by Firefox and Safari, it can still bring some performance benefits in some cases (if the resource is already cached, the browser will cancel the download, and servers will only trigger the push if conditions are ideal). AFAIK, all web servers supporting Server Push have a configuration option to disable it necessary. TL;DR: I wouldn't bother adding this attribute, as it's no longer part of any standard anyway. Regarding 103 Early Hints, I tried to use them in the original implementation of AssetMapper, unfortunately it's current not possible to use Early Hints to preload files used in importmap: #50476 However, now that the component supports CSS files, it should be possible to use Early Hints to preload CSS files. |
Thank you for that detailed clarification! I'll close this
Cool :). It won't be for 6.4, but we should think about this for the future. |
See conversation here - #51829 (comment)
I am not 100% sure this is needed - I'd love an HTTP/2 expert (perhaps dunglas?) to weigh in on this. Notes:
A) The purpose of the
Link
header is to hint to the browser as early as possible that a CSS file is needed. However, a web server MAY see this and perform a server push. That is NOT desired, as it would push the asset on every request, even after it is cached.B) So, adding
nopush
makes sense. But I'm not sure which web servers, in practice, are actually server-pushing at this time.Note: HTTP/2 server push is removed from Chrome - https://developer.chrome.com/blog/removing-push/ - which means they send a
SETTINGS_ENABLE_PUSH=0
setting when establishing the connection to ask the server to NOT push.So I think this is a good change... unless it's totally not needed because web servers aren't server-pushing already. And I don't know if that's the case.
Cheers!