Introduce analytics-chunks experiment#27605
Conversation
| return new AnalyticsConfig(this.element) | ||
| .loadConfig() | ||
| .then((config) => { | ||
| loadConfigDeferred.resolve(config); |
There was a problem hiding this comment.
Nit: It's generally better to resolve with a promise, instead of thening and resolving with a value. That way, rejected promise state will also be propagated (if it's possible, not sure here).
So, configPromise = new AC(…).loadConfig(); loadConfigDeferred.resolve(configPromise)
There was a problem hiding this comment.
Good point! Switched to resolve with a promise.
micajuine-ho
left a comment
There was a problem hiding this comment.
Thanks for this improvement! These look like some great changes 👍
| if ( | ||
| this.triggerCount_ < IMMEDIATE_TRIGGER_THRES || | ||
| !isExperimentOn(this.win_, 'analytics-chunks') | ||
| ) { |
There was a problem hiding this comment.
Probably worth it to leave a comment about why we want to execute the first IMMEDIATE_TRIGGER_THRES triggers immediately (aka increase active pageview ping).
| describes.integration( | ||
| 'basic pageview chunk', | ||
| { | ||
| body: ` | ||
| <script> | ||
| // initialize _cid cookie with a CLIENT_ID |
There was a problem hiding this comment.
Thanks for adding an integration test! :D
|
Please take a look again. I also chunk cookieWriter and sampleIn method in this PR. Thanks! |
|
@micajuine-ho Need your approval again on the bundle size check. |
micajuine-ho
left a comment
There was a problem hiding this comment.
cookieWriter and sampledIn chunking changes LGTM
Chunk by triggers and
#loadConfigsand#LinkerManagerOn Nokia device, with a config that has 20ish triggers
before
after
