Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bhauman opened this issue May 12, 2020 · 4 comments

Comments

@bhauman
Copy link

bhauman commented May 12, 2020

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
Copy link
Collaborator

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
Copy link
Author

bhauman commented May 13, 2020

Thanks!!

@laheadle
Copy link

Just bumped into this.

@laheadle
Copy link

my workaround:

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

(set! js/React react)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants