Skip to content
This repository has been archived by the owner on Jun 4, 2022. It is now read-only.

Add callback to lumo.build.api/build #265

Merged
merged 1 commit into from
Sep 15, 2017

Conversation

arichiardi
Copy link
Collaborator

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,
but includes a warning if that arity is invoked.

([source opts compiler-env]
(build source opts compiler-env (fn [& args] args)))
([source opts compiler-env cb]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was even wondering if we could accept the classic node (fn [err data] ...) callback so that you could promisify on it...but then I told myself: "KISS!!"

@arichiardi arichiardi force-pushed the build-callback branch 4 times, most recently from 075c4a0 to 57b4230 Compare September 15, 2017 00:38
@@ -2152,17 +2152,16 @@

(defn build
"Given a source which can be compiled, produce runnable JavaScript."
([source opts]
([source opts cb]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this still needs the 2-arity function

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Is it used? Do you prefer it? To avoid duplication, the default callback is passed in.i can add it back if you want to

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JVM CLJS has a 2-arity version. I think we should have one too

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok np will add it back

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.
@arichiardi
Copy link
Collaborator Author

Ok this should be ok now, no changes in lumo.closure anymore.

@anmonteiro anmonteiro merged commit 0959964 into anmonteiro:compiler-tweaks Sep 15, 2017
@arichiardi arichiardi deleted the build-callback branch September 15, 2017 06:53
anmonteiro pushed a commit that referenced this pull request Sep 16, 2017
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 pushed a commit that referenced this pull request Sep 17, 2017
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.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants