vouch-opensource / krell

Simple ClojureScript React Native Tooling
Eclipse Public License 1.0
675 stars 37 forks source link

Wait for all bootstrap paths to be cached #98

Closed olivergeorge closed 4 years ago

olivergeorge commented 4 years ago

Repeating this needs an empty cache and a physical android device. It's a race condition but it's clear from the code what needs to change.

waitForCache is not waiting for all the files which bootstrap needs before calling it.

The resulting error is "cannot read source of undefined". Screenshot attached to see the debugging which confirms what is described.

image

My suggested patch is

function waitForCore(cb) {
    // we only care if goog/base.js is actually in the cache, that's enough
    // to bootstrap regardless whether some things must be refetched
    if(KRELL_CACHE.ready
       && KRELL_CACHE.has(toPath("goog/base.js"))
       && KRELL_CACHE.has(toPath("goog/deps.js"))
       && KRELL_CACHE.has(toPath("cljs_deps.js"))
       && KRELL_CACHE.has(toPath("krell_repl_deps.js"))) {
        bootstrap();
        cb();
    } else if(typeof cljs !== 'undefined') {
        cb();
    } else {
        setTimeout(function() { waitForCore(cb); }, 250);
    }
}

Steps to repeat are fiddly but could probably be refined

I was able to see the error more easily after using the chrome debug tools to pause on the exception. To do that I needed to delay the cache initialisation by triggering tryConnection manually. It was all a bit messy but I suspect you know good ways to break on that exception and see the behaviour.

olivergeorge commented 4 years ago

I'm pretty confident this is an important fix.