This repository has been archived by the owner on Jun 4, 2022. It is now read-only.
Comprehensive enhancement to the build API #263
Merged
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
arichiardi
approved these changes
Sep 10, 2017
src/cljs/bundled/lumo/closure.cljs
Outdated
| (:file-min this))] | ||
| file-min | ||
| (:file this))] | ||
| (when (and file (not (js/$$LUMO_GLOBALS.path.isAbsolute file))) |
There was a problem hiding this comment.
I guess this guy can become just path now.
src/cljs/bundled/lumo/closure.cljs
Outdated
| @@ -1518,10 +1574,13 @@ | |||
| {:url (js/$$LUMO_GLOBALS.path.resolve out-file) | |||
| :out-file (.toString out-file)}))] | |||
| (when (and (not js-module?) | |||
| (or (not (js/$$LUMO_GLOBALS.fs.existsSync out-file)) | |||
| (and res (util/changed? out-file res)))) | |||
| (or (not (js/$$LUMO_GLOBALS.fs.existsSync out-file)) | |||
There was a problem hiding this comment.
This one too can change (minor of course, just marking)
| (ex-info (str "Preprocess symbol " preprocess " is not fully qualified") | ||
| {:file (:file js-module) | ||
| :preprocess preprocess}))) | ||
| preprocess-ns (symbol ns)] |
There was a problem hiding this comment.
TIL you can do this :)
src/cljs/snapshot/lumo/repl.cljs
Outdated
| ((if macros? | ||
| '#{cljs.core | ||
| (if macros? | ||
| ('#{cljs.core |
There was a problem hiding this comment.
A contains? would probably make this more readable
anmonteiro
force-pushed
the
compiler-tweaks
branch
from
4dc6021 to
3234870
Compare
arichiardi
reviewed
Sep 11, 2017
| (contains? (:js-dependency-index cenv) (name dep)) | ||
| (ana/node-module-dep? dep) | ||
| (ana/js-module-exists? (name dep)) | ||
| (deps/find-classpath-lib dep)) |
There was a problem hiding this comment.
Just curious, was there a problem with the old load?
There was a problem hiding this comment.
cljs.js/analyze-deps will try to load stuff that we don't know how to load (e.g. "module$...$left_pad") but it's safe to assume they exist.
same thing that's done here
There was a problem hiding this comment.
Oh cool thanks for explaining that
| (jar-resource? x) | ||
| (str "jar:file:" (.-jarPath x) "!/" (.-src x)) | ||
|
|
||
| :else (throw (js/Error. (str "Expected file, url, or string. Got " (pr-str x)))))) |
anmonteiro
force-pushed
the
compiler-tweaks
branch
2 times, most recently
from
3f1292c to
26c1680
Compare
|
I willmention the callback on build in the changelog, tomorrow though 😄 |
This change was made necessary by the fact that the bootstrap compiler is asynchronous by default. Therefore, it does not make sense to call lumo.build.api/build without a callback. The patch does not break old code, so it still contains the arity without cb, in which case it hooks a default callback that just returns the input.
anmonteiro
force-pushed
the
compiler-tweaks
branch
from
c2fa3fd to
0f58073
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
fixes #244
fixes #98