plexus / chestnut

Application template for Clojure + ClojureScript web apps
Eclipse Public License 1.0
1.32k stars 99 forks source link

[Version 0.16.0 (67651e9d)] index.html references inexistent .js file, figwheel version inconsistency #236

Closed TimoFreiberg closed 5 years ago

TimoFreiberg commented 6 years ago

Create a project without any options: lein new chestnut test-app

Version: Created with [Chestnut](http://plexus.github.io/chestnut/) 0.16.0 (67651e9d).

Run cider-jack-in-clojurescript in emacs:

Compiling build :app to "dev-target/public/js/compiled/test_app.js" from ["src/cljs" "src/cljc" "dev"]...
Successfully compiled build :app to "dev-target/public/js/compiled/test_app.js" in 7.272 seconds.

Same result with lein figwheel or cider-jack-in followed by (go) and (cljs-repl)

Open localhost:3449, see console: Loading failed for the <script> with source “http://localhost:3449/js/compiled/test_app.js”.

Fixed by replacing every occurrence of "dev-target" with "resources", but I assume the dev-target directory was there for a purpose :)

featheredtoast commented 6 years ago

Huh, that was working the last time I worked on it; Not sure what broke - I'll have a quick look soon, sorry about that!

dev-target is definitely on purpose. There are actually some really nasty and difficult to trace refresh issues when compiling with cljc files with cljs under a clj classpath. tldr, cljs compilation copies the sources over for use in source maps, including the cljc files. When you run a vanilla clojure refresh, it also picks up and attempts to reload the copied cljc files as well, resulting in some pretty broken repls.

That said, if you aren't using cljc you should be fine with just using resources as a compilation path.

featheredtoast commented 6 years ago

btw, localhost:3449 is a separate port that is serving figwheel; not your app -- you should be able to ignore that entirely. Your app is served on http://localhost:10555

Mind confirming that this is the case?

TimoFreiberg commented 6 years ago

Will do!

TimoFreiberg commented 6 years ago

Looks like I missed the Started test-app on http://localhost:10555 there. I still get an error on localhost:10555, but it seems like it's a completely different one:

Error: Error in component :app-root in system function (__meta,__extmap,__hash){
this.__meta = __meta;
this.__extmap = __extmap;
this.__hash = __hash;
this.cljs$lang$protocol_mask$partition0$ = 2229667594;
this.cljs$lang$protocol_mask$partition1$ = 139264;
} calling #'com.stuartsierra.component/start
core.cljs:11123:10
featheredtoast commented 6 years ago

Alright, so this is problematic, but this seems to be an issue with om upstream, as React.createClass has been deprecated/removed.

Quickly adding dependencies in project.clj for react 15.x allows this to compile again:

[cljsjs/react "15.0.0-0"]
[cljsjs/react-dom "15.0.0-0"]

This is om specific and should not affect other view layers like reagent or rum.

featheredtoast commented 6 years ago

Alternatively, I can also confirm that om beta-1 also works: [org.omcljs/om "1.0.0-beta1"]

plexus commented 6 years ago

Looking at https://github.com/omcljs/om/issues/899 it seems this is an issue with om.core ("classic" om), nobody has put in the work so far to make it compatible with React 16. There's a fork by @danielsz, but I don't see a PR to bring that back upstream.

We could pin React to React 15 when using classic Om, but maybe it's time to drop support for it altogether, if you're starting a new app I don't think it still makes sense to use classic Om. I would also prefer to switch to Reagent as the default. I think it's clear by now that in terms of mindshare and beginner friendliness Reagent has won.

TimoFreiberg commented 6 years ago

I would not mind either of those changes, but since I'm a total Clojurescript beginner I don't have a well-founded opinion.

featheredtoast commented 6 years ago

reagent as a sane default makes a whole lot of sense. I'd support making that the default and drop om classic.

featheredtoast commented 6 years ago

An update on this -- I'm making the decision to drop om, and have reagent be default in the latest release.

featheredtoast commented 5 years ago

Reagent is now the default rendering via #241