feat: wait load js for lms

[johanseto]

What's this PR do?

Fix cloudfront compatibility with missing js files loads.

This ensure to load the extra js files before xblock is called.

Before

Stage:

After

Stage:

Jira story

https://edunext.atlassian.net/browse/FUTUREX-1215

[johanseto]

@shadinaif @OmarIthawi Could you also check this PR to help fix SGA.

Issues -related

mitodl#336

PRs related

mitodl#319
mitodl#318

[andrey-canon]

I couldn't tests the CDN issue locally however the standard functionality worked as expected and the code LGTM

[shadinaif]

Thank you @johanseto.

I believe this resolves the issue. I guess the root cause is the fact that cloudfront is slow in many regions! all requests are almost guaranteed to finish before the cloudfront request. The only problem that I expect is that it'll make the page stuck in loading state for up to 10 seconds (sometimes). It's a problem with cloudfront itself, the file takes a lot of time to load unless it's cached by the browser

[OmarIthawi]

Thanks a lot @johanseto.

I've suggested few refactorings because the code might work well in most cases but fail unexpectedly due to race conditions.

Making the code more solid and more handled is good and should probably be contributed upstream.

That being said, if cloudfront is an issue in the middleeast and your region, let's use a better CDN or no CDN at all that stays an issue. @shadinaif do you have any source about the issues you've mentioned regarding cloudflare or that's an experience you've faced?

[OmarIthawi]

If async resolves the issue, then let's keep the change to its minimum amount.

      .attr('async', true)

[johanseto]

That doesn't resolve the problem; we need to wait until those scripts are loaded. So we need to use the onload, I tried with jquery approach, but it doesn't work

[OmarIthawi]

Suggested change

	async function StaffGradedAssignmentXBlock(runtime, element) {
	function StaffGradedAssignmentXBlock(runtime, element) {

We have two asyncs here, this async function and the scripts are async.

Are we sure we need both?

The LMS XBlock runtime (which calls this funtion) may not support promises and this might crash to unhandled promise error.

If async is a must, we must encapsulate the async logic inside StaffGradedAssignmentXBlock to ensure the promise is handled properly, instead of making the StaffGradedAssignmentXBlock itself async.

[johanseto]

Knowing your concerns now, I prefer to apply this change to remove the general async declaration. b776991

[johanseto]

@OmarIthawi

[johanseto]

Thank you @johanseto.

I believe this resolves the issue. I guess the root cause is the fact that cloudfront is slow in many regions! all requests are almost guaranteed to finish before the cloudfront request. The only problem that I expect is that it'll make the page stuck in loading state for up to 10 seconds (sometimes). It's a problem with cloudfront itself, the file takes a lot of time to load unless it's cached by the browser

Even cached in the browser the error persists, when you reload test_url. Also CDN idea is to load fast with Cloudfront without thinking about the region. If you check here, the timing is good, I mean 70ms.

Anyway, the problem exists maybe because the script URL is using and URL different than the window URL. (
https://dg4ogka6-test.cloudfront.net/static/js/vendor/jQuery-File-Upload/js/jquery.fileupload.js) and the js block code continues calling xblock without loading the libraries...

edx-sga/edx_sga/static/js/src/edx_sga.js

Line 513 in 2888ae4

xblock($, _);

[OmarIthawi]

Thanks @johanseto, no objections from my end. Thanks!!