-
Notifications
You must be signed in to change notification settings - Fork 265
WS-1816: Ensure correct Piano L2 tracking for Magyarul & Romania #13587
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
WS-1816: Ensure correct Piano L2 tracking for Magyarul & Romania #13587
Conversation
…ure-correct-L2-tracking-for-Magyarul-Romania
|
The failing |
| }} | ||
| /> | ||
| <> | ||
| <link href="https://mybbc-analytics.files.bbci.co.uk" rel="preconnect" /> |
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.
We have the Link header setting here: https://github.com/bbc/simorgh/blob/b45d6729d7dd7625a32e08bd356e1daa7cc54f06/ws-nextjs-app/utilities/addLinkHeader/index.ts
is it worth adding the domain to the list of origins to add to the Link header?
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.
Thanks for giving this a consideration Aaron.
I've made this change here to ensure analytics resources are
co-located. Additionally, having this clause here means the
pre-connection only happens on Canonical where Reverb is used.
Is the change you've recommended covered by the fact that ReverbTemplate
is used in _document.page.tsx?
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.
The addLinkHeader function will add the headers to all pages and app platforms, so canonical, AMP and Lite. If its best not to have it on those platforms then it may be best keeping it in ReverbTemplate.
The Link header will hint to the browser earlier though, so it may be more performant.
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.
Thanks for the context Aaron.
Let me see if I can add https://mybbc-analytics.files.bbci.co.uk to the Link
header for Canonical and app. This way, Lite and AMP won't need to establish the
connection to a domain that's not in use on the page.
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.
Hey @amoore108
The resource hints changes are now in place. I've updated the header
to use dns-prefetch for
<https://a1.api.bbc.co.uk>; rel="dns-prefetch",
<https://ping.chartbeat.net>; rel="dns-prefetch",
<https://mybbc-analytics.files.bbci.co.uk>; rel="dns-prefetch",
and preconnect for
<https://ichef.bbci.co.uk>; rel="preconnect"; crossorigin,
<https://static.files.bbci.co.uk>; rel="preconnect"; crossorigin,
Below are references to the changes I've made.
Resource Hints Array
https://github.com/bbc/simorgh/pull/13587/files#diff-15710eab1fc3b7ad35a96c08c970f92696d92b8eaecb60e437b6447ba67cb26f
I've cc'ed you on the Slack thread with the rationale behind the
resource hints changes I've made.
Thanks. 😄
…' of github.com:bbc/simorgh into WS-1816-ensure-correct-L2-tracking-for-Magyarul-Romania
amoore108
left a comment
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.
Thanks for the changes! LGTM.
Resolves JIRA: WS-1816
Summary
Update the Reverb source used to point to the centrally provided Reverb library.
Code changes
SIMORGH_REVERB_SOURCEto point tohttps://mybbc-analytics.files.bbci.co.uk/reverb-client-js/reverb-3.10.2.jsacross all environments.Developer Checklist
Testing
Ready-For-Test, Local)Ready-For-Test, Test)Ready-For-Test, Preview)Ready-For-Test, Live)Additional Testing Steps
Networktab of the Developer Tools, examine the Reverb source.https://mybbc-analytics.files.bbci.co.uk/reverb-client-js/reverb-3.10.2.jss2should read134for MAGYARUL and136for ROMANIAN.Useful Links