@dnolen the MAX_SAFE_INTEGER here is probably an oversight, or is it on purpose? https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/spec.cljs#L638

because Number.MAX_SAFE_INTEGER is ES6, I think

(you do define a MAX_INT at the beginning of the namespace, so I'm leaning towards oversight)

@anmonteiro we can’t use ES6

right, so probably a bug

need to change that to the MAX_INT constant defined at the top of the namespace

right sorry yeah I think just did that one quickly

@dnolen I'm porting the perf tweaks, should I change it as part of this patch or in a separate one?

and forgot to change it later

@anmonteiro fine if in the same patch

OK, will do

@dnolen attached a patch to the perf tweaks JIRA ticket: http://dev.clojure.org/jira/browse/CLJS-1768 as noted in the comment, I've pushed a branch with the 4 different commits to make it easier to review: https://github.com/anmonteiro/clojurescript/tree/spec-perf-tweaks

@anmonteiro thanks!

@dnolen the following ticket is assigned to you, are you planning to fix it? http://dev.clojure.org/jira/browse/CLJS-1702

@anmonteiro if you want to take it go for it

👍

@dnolen as I mentioned in the backlog, I think this is not a bug: http://dev.clojure.org/jira/browse/CLJS-1622

he's trying to use with-redefs with static-fns true

as far as I understand, this is only supported if the function is marked ^:dynamic, correct?

yes closed

@dnolen should we just remove this? https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L2291-L2302

we don't really use it anymore now that require is a thing

probably would mean breaking changes for tooling... hrm

@anmonteiro how?

was about to say yes as long as it doesn’t break anything internally

externally no one should have been relying on that

it doesn't internally

if they are - they should expect to break

OK I'm OK with that

I think e.g. Planck implements require through this mechanism

but Planck is tied to a specific ClojureScript version so it can just choose to upgrade whenever

yes

Right—don't worry about Planck; it will be updated to the new macros instead of REPL specials. Besides, it's ok to break Planck.

(It's not using public API.)

submitted in http://dev.clojure.org/jira/browse/CLJS-1842

tested REPLs (including Browser), and ran tests

going to look at http://dev.clojure.org/jira/browse/CLJS-1666 next ("Flag to optionally disable transit analysis cache encoding"). what should this compiler option be called?

:cache-analysis-format, should support 2 values :edn & :transit

@dnolen OK sounds good. default is the current impl? i.e. :transit if the lib is in the classpath

yes

@dnolen http://dev.clojure.org/jira/browse/CLJS-1666 just attached a patch to the transit thing

@anmonteiro probably want to provide :edn as default for the 3 argument case no? that and add an explicit case for :edn, and a throwing case for unknown format?

@dnolen why should we default to :edn in the 3 argument case?

I thought we wanted to default to Transit

@anmonteiro hrm sorry you’re right but I think we need this to be less implicit then

that is the logic for choosing the format shouldn’t be hard coded into the final arity

but before

then you have the option to fully control the beahvior

right, so we're forced to change the API, I believe

and I think we still want to throw if we can’t determine the format

yeah, I have no arguments against that

@anmonteiro yes so you’ll have to touch a couple of other things

but I’d rather we just fix it

no problem touching other stuff, just wanted to make sure

thanks for the feedback, incorporating it now

thanks

@anmonteiro @dnolen added a comment to http://dev.clojure.org/jira/browse/CLJS-1666 ... just my 2 cents, gotta sleep now. be back tomorrow.