tonsky / rum

Simple, decomplected, isomorphic HTML UI library for Clojure and ClojureScript
Eclipse Public License 1.0
1.78k stars 124 forks source link

use react and react-dom instead cljsjs.react and cljsjs.react.dom to support new cljs :bundle target #211

Open bhauman opened 4 years ago

bhauman commented 4 years ago

The new cljs :bundle target allows pretty seamless npm and webpack integration for CLJS projects.

I just tried to fix the Rum part of the lein-figwheel template to operate with this new target and it didn't work. This is because Rum is still using :require [cljs.react][cljs.react.dom] while the same cljsjs deps can be required via :require [react][react-dom] which also allows developers to provide react and react-dom via npm instead.

When changing this way of requiring libs one also needs to refer to the ns as the js object. For example one would use react-dom/render instead of js/ReactDOM.

Reagent has already adopted this convention.

There may be a reason that you are doing things this way but if possible it would be pretty cool to allow react to be provided by npm deps.

roman01la commented 4 years ago

Yep, this is on the roadmap. I've seen that Juho already worked on a PR a while back. I'll prioritize this to unblock figwheel template sooner.

bhauman commented 4 years ago

Thanks!!

laheadle commented 4 years ago

Just bumped into this.

laheadle commented 4 years ago

my workaround:

(ns cljsjs.react
  (:require [react]))

(set! js/React react)