Multimethods #4
Open
Multimethods #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This trival change is to support holding on to the original krei file for passing to build components, so debugging information has access to the krei-file resource URL.
This is a backward compatible change that retains the existing functionality while preparing the way for new 'providers' of build functionality, such as shadow-cljs and JIT image resizing.
|
I forgot to mention that the result of the |
This was an oversight.
This is a new design whereby the build configuration is passed to
krei/kick, for example, from an Aero configuration.
```clojure
{:default {}
:dev
{:kick/builder
{:kick.builder/target "target"
:kick/sass {:sources ["public/app.scss"]}
:kick/figwheel {:builds [{:id "main"
:figwheel {:on-jsload "edge.main/init"}
:compiler {:main edge.main
:output-to "public/edge.js"
:output-dir "public/edge.out"
:asset-path "/public/edge.out"}}]}}}}
```
The watch and returning close function correspond directly to
`integrant.core/init-key` and `integrant.core/halt-key!`, although
there is no integrant dependency from krei/kick
IMPORTANT: Since krei can _no longer build independently_ the
production build (called by main) has been removed entirely and
consuming projects now need to provide this functionality.
|
OK, this is now ready to review. There is more context and design rationale in the individual commit messages. I've gone for a BYOD (bring your own dependency) policy for the individual providers, and none are therefore privileged. I'm planning to add shadow-cljs in due course. The point of this BYOD policy is to avoid dependency bloat (avoid paying for the cost of dependencies you don't use). One strange thing is that Edge seems to 'work' (build the sass) without seeming to bring in deraen/sass4clj {:mvn/version "0.3.1"}(not sure what's going on there, but it doesn't affect this review). |
SevereOverfl0w
requested changes
Aug 3, 2018
src/io/dominic/krei/alpha/core.clj
Outdated
| [io.dominic.krei.alpha.impl.debounce :as krei.debounce])) | ||
| [clojure.java.classpath :as classpath] | ||
| [clojure.string :as string] | ||
| [clojure.tools.logging :as log] |
There was a problem hiding this comment.
tools.logging isn't a dependency of the project. So this commit only compiles by coincidence.
src/io/dominic/krei/alpha/core.clj
Outdated
| (map #(update-in % [:compiler :output-to] (comp str target-relative)))) | ||
| krei-files)})) | ||
| (build-sass) | ||
| ;; @dmc what's this? still something we need? |
There was a problem hiding this comment.
which this? :p Presuming you mean (run! dirwatch/close-watcher krei-builders) it takes care of shutting down the file watchers.
```
---- Exception ----
No implementation of method: :-find-sources of protocol: #'cljs.closure/Compilable found for class: cljs.build.api$inputs$reify__6688
---- Exception Stack Trace ----
java.lang.IllegalArgumentException: No implementation of method: :-find-sources of protocol: #'cljs.closure/Compilable found for class: cljs.build.api$inputs$reify__6688
at clojure.core$_cache_protocol_fn.invokeStatic (core_deftype.clj:583)
clojure.core$_cache_protocol_fn.invoke (core_deftype.clj:575)
cljs.closure$eval45293$fn__45307$G__45284__45314.invoke (cljs_closure.clj:475)
cljs.closure$build.invokeStatic (cljs_closure.clj:2707)
cljs.closure$build.invoke (cljs_closure.clj:2672)
cljs.build.api$build.invokeStatic (api.clj:208)
cljs.build.api$build.invoke (api.clj:189)
figwheel_sidecar.components.cljs_autobuild$cljs_build.invokeStatic (cljs_autobuild.clj:28)
```
| @@ -0,0 +1,36 @@ | |||
| ;; Suggested dependencies: | |||
| ;; thheller/shadow-cljs {:mvn/version "2.0.123"} | |||
| ;; As there's a bug in tools.deps.alpha (TDEPS-26), also add: | |||
There was a problem hiding this comment.
TDEPS-26 is fixed https://dev.clojure.org/jira/browse/TDEPS-26
This calls for configurability.
This is to ensure that unpacked production instances will build their own CSS prior to starting.
malcolmsparks
force-pushed
the
master
branch
2 times, most recently
from
e71d0d6
to
b720686
Compare
malcolmsparks
force-pushed
the
master
branch
from
b720686
to
158695f
Compare
Syntax has changed - see Edge
This makes developing with figwheel possible without loading kick in the context of a project.
This is a convention set by Asciidoctor.
This BINARY INCOMPATIBLE change is to allow finer control of the target filename. This is required by Edge to place generated CSS files into a directory named 'public'. Given the fast-moving alpha status of Kick, with the ease of fixing its (few) current dependants, simplicity has been chosen over binary compatibility.
This prevents a bug which happens as a combination of: - Kick being a git dependency - Kick allowing users to bring their own dependencies - tools.namespace loading all namespaces which are in directories This guard prevents the namespace from exploding when required without the required dependency present. This is not completely ideal, due to the confusion it will push down onto users when namespaces unexpectedly don't work.
This causes a compilation error when starting a clojurescript REPL, as figwheel-main attempts to compile the code in watch-dirs then.
You need the :output-dir to be produced when source maps are enabled.
clj no longer produces a clojure.libfile, and kick needs this to remove itself from the classpath when using figwheel-main. Newer versions of clj instead provide a clojure.basis, which is vaguely the same as the old basis. Supports both in order to handle the transition period of updates.
Update to clojure.basis
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tested against the JUXT website.