Conversation
Fixed a race condition when loading map for the first time.
There was a problem hiding this comment.
Race conditions can occur because of load order (which script was transferred the fastest), but that's what defer was supposed to prevent by giving the scripts an explicitly different load order. So I looked around a bit for more specific information.
We assumed defer would queue execution of that script after non-deferred scripts. Unfortunately that's not the case, defer scripts are run before the DOMContentLoaded event is fired.
The body's onLoad event, however, fires only when a page is fully loaded. This causes the inconsistent execution order.
And I believe your example of disabling clustering isn't a good one, since custom.js.example includes code that checks whether the library was already loaded or not. If it was, it calls the existing instance directly.
I've included 2 possible fixes in my review comment. 👇
| // wrapper function for real initMap (_initMap) to | ||
| // avoid race conditions with custom.js | ||
| function initMap() { // eslint-disable-line no-unused-vars | ||
| setTimeout(_initMap, 100) |
There was a problem hiding this comment.
Setting a 100ms timeout is a guess for execution order (an arbitrary delay). Instead, you have several options that guarantee execution order without delay:
- Replace jQuery's onLoad from custom.js with a self-executing function.
- Define a
customJsLoaded(not the best naming) incustom.jsand check for its existence inmap.js. If it exists, call it before doing any initialization so it can properly set all defaults.
| <script src="{{ url_for('static', filename='dist/js/app.min.js').lstrip('/') }}"></script> | ||
| <script src="{{ url_for('static', filename='dist/js/map.common.min.js').lstrip('/') }}"></script> | ||
| <!-- Load custom JS before map.js so it can change Store defaults. --> | ||
| <!-- Custom JS is loaded but not actually executed prior to map.js as all its code is executed in jQuerys onload function. --> |
There was a problem hiding this comment.
Read my other comment, and put this message back as it was.
sebastienvercammen
added
pending changes
and removed
to be checked
awaiting review
labels
|
@farstarss Are you still interested in updating this PR? |
2 participants
Fixed a race condition when loading map for the first time.
Description
Although custom.js is listed above map.js in map.html it's not being executed prior to map.js since the settings inside custom.js are set on page load - which is fine as they depend on Store which is defined in map.js.
The issue is that the google maps script is also going to be executed on page load (=>
deferattribute) which results in a race condition between custom.js and google maps script callinginitMap()and sometimes results in custom settings being ignored and default settings being used.To reproduce this just disable clustering in custom.js and load the map in a private tab: You'll see that clustering sometimes happens and sometimes it doesn't.
Motivation and Context
Race conditions should always be addressed and fixed.
How Has This Been Tested?
Locally.
Screenshots (if appropriate):
Types of changes
Checklist: