bhauman / lein-figwheel

Figwheel builds your ClojureScript code and hot loads it into the browser as you are coding!
Eclipse Public License 1.0
2.88k stars 210 forks source link

Figwheel crash calling js\require in complex circumstances #649

Open deg opened 6 years ago

deg commented 6 years ago

I'm seeing what looks like a Figwheel crash:

[exp] undefined is not an object (evaluating 'receipts_native.core.init.call')
* js/figwheel-bridge.js:117:14 in <unknown>
- node_modules/promise/setimmediate/core.js:37:14 in tryCallOne
- node_modules/promise/setimmediate/core.js:123:25 in <unknown>
- ... 10 more stack frames from framework internals

[exp] Unknown named module: 'native-base'
- node_modules/metro-bundler/src/Resolver/polyfills/require.js:96:12 in _require
* js/figwheel-bridge.js:162:26 in require
* js/figwheel-bridge.js:117:14 in <unknown>
- node_modules/promise/setimmediate/core.js:37:14 in tryCallOne
- node_modules/promise/setimmediate/core.js:123:25 in <unknown>
- ... 10 more stack frames from framework internals

This is triggered by trying to js\require a module from code generated by a macro.

I've branched my project to create a small example that demonstrates the issue. See https://github.com/deg/receipts-native/tree/figwheel-require. The key lines are https://github.com/deg/receipts-native/blob/e39a22b39c69d28ae153b84f028dab304e930279/src/receipts_native/macros.cljc#L23-L34

#?(:clj
   (defmacro def-native-components
     "Extract one or more React classes from a JavaScript module. Adapt them as Reagent components and name them.

      [TODO] Weird bug that the js/require here seems to crash figwheel, unless the same module was previously
      require'd from .cljs. If this bug is real, I'll probably have to remove the require from this macro."
     [module-name names]
     (let [module (gensym "module-")]
       `(let [~module (js/require ~module-name)]
          ~@(map (fn [name]
                   `(def ~name (get-class ~module ~(str name))))
                   names)))))

As it stands now, my code is minimal. The one complication is that it requires an Expo React Native setup. I hope this is reduced to something small enough that you can see the problem. If not, I can try to create a test case that does not need React Native (though this is the first context in which I've had to use js\require, so I'll be treading on some unfamiliar ground).

deg commented 6 years ago

Looks like defmacro was a red herring. I don't know much more yet, but wanted to give you an alert that I don't fully understand the problem yet.

It seems to be enough to have the js/require in some unusual position, rather than at top-level. I'm still not certain what I'm seeing.

Does this ring any bells? Is there something magical about how js/require is handled?

deg commented 6 years ago

Well, defmacro was a red herring. I've simplified the code, and can reproduce the crash just wrapping the js/require in a let. No need to define a macro, or any other complexity.

Still in the same branch: https://github.com/deg/receipts-native/tree/figwheel-require, and the relevant code is now at https://github.com/deg/receipts-native/blob/figwheel-require/src/receipts_native/core.cljs#L11-L26:

;;; Restoring this next line removes the crash.
;;; It seems that Figwheel (or something) is unhappy when module is first required in the
;;; expansion of a macro???

;; (defonce native-base (js/require "native-base"))

(defn get-class
       "Extract React class from JavaScript module and adapt as Reagent component."
       [module class-name]
     (r/adapt-react-class (aget module class-name)))

(let [module (js/require "native-base")]
  (def Body      (get-class module "Body"))
  (def Container (get-class module "Container"))
  (def Header    (get-class module "Header"))
  (def Title     (get-class module "Title")))

I've still only reproduced this in the context of Expo and React-Native. I'll try to find a simpler case.