Moving CDN PreFetch to kick off prior to initialize call and added timeout logic

[jadahiya-MSFT]

For more information about how to contribute to this repo, visit this page.

Description

This change was made to address customer feedback that the network call to acquire CDN resources was leading to initialization time outs. We aim to address this concern by decoupling that call.

Summarize the changes, including the goals and reasons for this change. Be sure to call out any technical or behavior changes that reviewers should be aware of.

If this Pull Request should close/resolve any issues when merged, use the special syntax for that here.

Main changes in the PR:

1. Decoupled CDN prefetch from initialization flow by moving logic into prefetch.ts file to execute a global fetch upon library load. Moved into a separate file since global functions cause side effects that can't be treeshakeable.
2. Added a timeout of 1.5 seconds for CDN resource acquisition before defaulting back to fallback list.
3. Fixed a typo in jest-setup.cjs file
4. Removed fetch polyfill rom test\utils.ts file and moved into a new setupTest.ts file that is used during jest setup.

Validation

Validation performed:

1. Tested locally using test app with Orange
2. Ran E2E and Unit tests
3. Monitored network timings of moving prefetch to Global space versus during the initialization flow and increased speeds for CDN asset acquisition on lower end networks with 3G and slow 4G speeds. Taking now at most 600ms to complete on both of those network configurations.
4. Confirmed that timeout logic for CDN acquisition works by intentionally blocking network call for 1.5 seconds.

Unit Tests added:

Unit tests are required for all changes. If no unit tests were added as part of this change, please explain why they aren't necessary.

1. Added setupTests.ts file to tests to enable global fetch polyfill for Jest tests.

[TrevorJoelHarris]

Now that this is called in the global namespace this file has a side effect that it didn't used to have (loading the file in memory now automatically makes a call to the CDN). Can you verify with Noah that we don't need to mark this file in any particular way to prevent it from being treeshaken out?

[jadahiya-MSFT]

Good call, I'll follow up with Noah on this and if there's any issue I'll move this call into a separate funciton.

[jadahiya-MSFT]

Followed up with Noah, this will in fact introduce an unintended side effect that would make app.ts not treeshake-able. I'll be pushing out a change that will mitigate this or lessen the impact of the side effect.

[AE-MS]

Is there something we can do so issues like this are apparent to developers who introduce them without requiring @TrevorJoelHarris or @noahdarveau-MSFT to be manually looking for them in code reviews? Certainly I wouldn't have realized this.

[TrevorJoelHarris]

Hmmm, that's a good question. I'll think about it (and maybe ask @noahdarveau-MSFT). In general, any function calls in the global namespace are "side effects" if they cause some state somewhere to change because those functions will just get called assuming the entire module doesn't get tree-shaken out and removed. Off the top of my head I'm not sure there's a way to "detect" that.

[TrevorJoelHarris]

Okay, I am pretty sure that we could make a linter rule to detect global function calls that would show a warning saying something like "This file should probably be added to the sideEffects array in package.json. Once you've done that (or verified that you don't need to) please suppress this warning."

[TrevorJoelHarris]

Left a few questions

[AE-MS]

async function getValidOriginsListFromCDN(): Promise<string[]> {

If this function gets called a second time while the first fetch is still waiting, will it kick off a second CDN network request? Is that... okay? I can't think of a reason why it would cause "damage" but it is maybe unnecessary?

¯\_(ツ)_/¯
``` #Closed

---
Refers to: packages/teams-js/src/internal/validOrigins.ts:18 in 74a6e89. [](commit_id = 74a6e89d8f652d51f393b5320ff0a58ef8975106, deletion_comment = False)

[AE-MS]

🕐

[AE-MS]