-
Notifications
You must be signed in to change notification settings - Fork 243
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
Filter duplicate extensions by src URL #1192
Changes from 4 commits
2efc49d
d96effd
118fab7
ec427be
0ce925f
85396ef
63ef718
68804c2
3192282
6901062
f8d0d1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,9 +25,9 @@ class HeadNodes { | |
this._metaCharset = null; | ||
this._scriptAmpEngine = null; | ||
this._metaOther = []; | ||
this._resourceHintLinks = []; | ||
this._scriptRenderDelayingExtensions = []; | ||
this._scriptNonRenderDelayingExtensions = []; | ||
this._resourceHintLinks = []; | ||
this._linkIcons = []; | ||
this._styleAmpCustom = null; | ||
this._linkStylesheetsBeforeAmpCustom = []; | ||
|
@@ -50,24 +50,24 @@ class HeadNodes { | |
} | ||
|
||
_removeDuplicateCustomExtensions(extensions) { | ||
const nodesByName = new Map(); | ||
const nodesBySrc = new Map(); | ||
for (const node of extensions) { | ||
const name = this._getName(node); | ||
nodesByName.set(name, node); | ||
const src = node.attribs['src']; | ||
nodesBySrc.set(src, node); | ||
} | ||
return Array.from(nodesByName.values()); | ||
return Array.from(nodesBySrc.values()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took an alternative approach to this in PHP, to do the deduplication in the This also accounts for |
||
|
||
appendToHead(head) { | ||
appendChild(head, this._metaCharset); | ||
appendAll(head, this._metaOther); | ||
appendAll(head, this._resourceHintLinks); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point! done |
||
appendChild(head, this._linkStyleAmpRuntime); | ||
appendChild(head, this._styleAmpRuntime); | ||
appendAll(head, this._metaOther); | ||
appendChild(head, this._scriptAmpEngine); | ||
appendAll(head, this._scriptRenderDelayingExtensions); | ||
appendAll(head, this._scriptNonRenderDelayingExtensions); | ||
appendAll(head, this._linkIcons); | ||
appendAll(head, this._resourceHintLinks); | ||
appendAll(head, this._linkStylesheetsBeforeAmpCustom); | ||
appendChild(head, this._styleAmpCustom); | ||
appendAll(head, this._others); | ||
|
@@ -160,7 +160,13 @@ class HeadNodes { | |
return; | ||
} | ||
|
||
if (rel === 'preload' || rel === 'prefetch' || rel === 'dns-prefetch' || rel === 'preconnect') { | ||
if ( | ||
rel === 'preload' || | ||
rel === 'prefetch' || | ||
rel === 'dns-prefetch' || | ||
rel === 'preconnect' || | ||
rel == 'modulepreload' | ||
) { | ||
this._resourceHintLinks.push(node); | ||
return; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this remove the host to make sure a same extension is not downloaded twice from two different hosts?