vouch-opensource / krell

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

goog.debug.freeze is not a function #100

Closed olivergeorge closed 3 years ago

olivergeorge commented 4 years ago

For some reason target/goog/debug/debug.js doesn't seem to be loaded before target/goog/events/browserevent.js on android. This leads to a "Failed to load file" error.

Test setup

(ns awesome-project.core
  (:require [reagent.core :as r]
            [reagent.react-native :as rn]
            [goog.events.BrowserEvent]))   ; <--- Add this

Steps to repeat

Expect: Clean boot

Actual: console error and logging by Krell indicating "Failed to load file"

07-17 08:23:20.138 31267 31457 E ReactNativeJS: TypeError: goog.debug.freeze is not a function. (In 'goog.debug.freeze([
07-17 08:23:20.138 31267 31457 E ReactNativeJS:   1,  // LEFT
07-17 08:23:20.138 31267 31457 E ReactNativeJS:   4,  // MIDDLE
07-17 08:23:20.138 31267 31457 E ReactNativeJS:   2   // RIGHT
07-17 08:23:20.138 31267 31457 E ReactNativeJS: ])', 'goog.debug.freeze' is undefined)
07-17 08:23:20.159 31267 31457 I ReactNativeJS: 'Failed to load file:', 'target/goog/events/browserevent.js'

Related experimentation

FSpoering commented 4 years ago

I got to that stage when i was adding re-frame to the project. but i did not investigate it further.

olivergeorge commented 4 years ago

Failing consistently on Android simulator now so I'm hopeful I'll be able to find a simple repro (or fix a bug elsewhere).

Seems like the app isn't requesting goog/goog/debug.js before trying to load target/goog/events/browserevent.js

olivergeorge commented 4 years ago

Yep, seems repeatable by simply requiring goog.events.BrowserEvent.

Updated description to match.

olivergeorge commented 4 years ago

I traced the code and found that the app was reaching CLOSURE_IMPORT_SCRIPT via loadFile for debug/debug.js but the server didn't seem to recieve the loadFile request. I could see the request before and after getting through so I'm confident the logging was in place.

olivergeorge commented 4 years ago

I think perhaps exists_ test is getting confused for the goog.debug case.

olivergeorge commented 4 years ago

In this example target/goog/debug/error.js and target/goog/debug/errorcontext.js are being loaded first. They don't depend on goog.debug.

By the time target/goog/events/browserevent.js is being evaluated the goog.debug object exists.

As a result the exists_ testing for goog.debug decides it must already be loaded and filters it out.

swannodette commented 4 years ago

Thanks for the detailed reproducer. Will take a look.

olivergeorge commented 4 years ago

Few questions which came out of discussing the issue...

Q: Should goog/debug/debug.js be loaded before target/goog/debug/errorcontext.js by convention since goog.debug.errorcontext is "under" goog.debug?

Neither android or iOS do this. Based on that it could be the underlying issue is common to both (root bug) and exists_ doesn't need to change.

Q: If the exists_ test is necessary to avoid double loading namespaces then why doesn't iOS need an equivalent exists_ test?

My suspicion is that either exists_ is needed for both (a second bug) or is unnecessary.

bpringe commented 3 years ago

I got to that stage when i was adding re-frame to the project.

I just ran into this as well after adding re-frame to my project. After making a code change and the app reloading, the error stopped being logged, and things seem to be working okay.