Dynamically import consented code #1397
Conversation
🦋 Changeset detectedLatest commit: adc6650 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Jakeii
added
the
[beta] @guardian/commercial
|
🚀 0.0.0-beta-20240530094903 published to npm as a beta release |
Jakeii
changed the title
Jakeii
added
[beta] @guardian/commercial
|
🚀 0.0.0-beta-20240530130323 published to npm as a beta release |
| @@ -21,16 +34,9 @@ const chooseAdvertisingTag = async () => { | |||
| './init/consentless' | |||
| ).then(({ bootConsentless }) => bootConsentless(consentState)); | |||
| } else { | |||
| bootCommercialWhenReady(); | |||
| void import( | |||
| /* webpackChunkName: "consented" */ | |||
There was a problem hiding this comment.
Whilst this may decrease the size of the bundles it could introduce a delay to commercial-start for consented as we are introducing a waterfall loading process. I think its worth measuring or running as an ab-test.
There was a problem hiding this comment.
We will be monitoring our commercial metrics of course to do pre/post analysis, an ab test is likely to be produce clearer results although with the complexity of serving different bundles?
There was a problem hiding this comment.
Okay great as long we are tracking the impact to commercial-start for consented. It will be interesting to see if there is any impact to mobile devices which are usually more network constrained.
There was a problem hiding this comment.
The code looks good and thank you @Jakeii for answering my questions in DM!
I can see from the chat on the PR we will merge that change and keep an eye on the commercial-start metric for consented but is there any further testing that could be done before we merge?
Maybe this would build more confidence? #1401 (comment) |
Yes, definitely that would make things better. I assume the PR you shared is going to be merged before this one so we can see the outcome. Correct? |
Ad load time test resultsFor Test conditions:
|
What does this change?
Dynamically import our consented code from the entrypoint as well as our consented code which is already dynamically imported.
Why?
This makes a small improvement to the amount of code transferred in our consentless environment.
The increase in the number of chunks may also help with performance as they can now load in parallel.