Fork me on GitHub
#cljs-dev
<
2017-09-15
>
dnolen12:09:24

@bhauman yeah I don’t think we use that anymore

dnolen12:09:18

but I think you’re saying that Figwheel is going to be lot zippier now 🙂

dnolen12:09:07

@lxsameer we don’t want that try/catch in there - I just don’t understand what that’s accomplishing

lxsameer12:09:44

@dnolen let me show you some code 🙂

dnolen12:09:13

sure but I might not care about your examples

lxsameer12:09:53

that's true but the point of a predicate is to either return true of false, and not to throw an exception

dnolen12:09:08

we don’t do this type of thing in ClojureScript

dnolen12:09:18

please look at how things are done

dnolen12:09:24

we’re not making an exception for this predicate

dnolen12:09:39

I added my comments on the ticket

lxsameer12:09:06

sure, thanks 🙂

lxsameer12:09:29

@dnolen I'm writing down my experience about contributing to cljs so i can make an article from them later for http://clojurescript.org, so

lxsameer12:09:02

can you please point out the main problems about that try/catch ? i want to add them for later reference

dnolen12:09:15

this doesn’t need go anywhere

dnolen12:09:18

code review is code review

lxsameer12:09:42

alright. thanks

lxsameer13:09:41

I assume there is no specific rule about trailing space. is that correct ?

dnolen13:09:10

no but irrelevant whitespace changes are not desirable

dnolen13:09:06

@lxsameer upstream externs

lxsameer13:09:03

like externs distributed with closure itself ?

dnolen13:09:24

no like from upstream dependencies

dnolen13:09:55

deps.cljs is a special EDN file that can be contained in JAR which define foreign libs, externs, etc.

dnolen13:09:04

we find them all and use all of their contents

lxsameer13:09:17

aha, got it thanks

lxsameer13:09:32

what about infer-externs

favila13:09:22

Try/catch is because 22 instanceof goog.Uri throws TypeError (which surprised me). Would need a typeof 22 === "object" guard

dnolen13:09:29

yeah not gonna worry about that for this patch

dnolen13:09:36

@lxsameer inferred externs

dnolen13:09:04

there’s a language feature to infer externs on behalf of the user via type hinting

lxsameer13:09:08

@dnolen thanks, i need to learn more about infer externs

lxsameer13:09:42

@favila it was my mistake, the instance is working just fine

dnolen20:09:54

@ambrosebs your AST :const patch needs a rebase

anmonteiro20:09:56

but I accidentally pasted *warn-on-reflection* in CLJS tests

anmonteiro20:09:00

that fixes it

dnolen20:09:36

ah, got it thanks

dnolen20:09:59

@anmonteiro on master I keep getting a weird warning now while installing Node.js deps

dnolen20:09:06

Installing Node.js dependencies events.js:160 throw er; // Unhandled ‘error’ event ^ Error: Can’t resolve ‘./calculator_amd’ in ‘/Users/davidnolen/development/clojure/clojurescript’

anmonteiro20:09:25

safe to ignore

dnolen20:09:35

ah k but why does it do that again?

anmonteiro20:09:38

let me dig the reason

anmonteiro20:09:13

we now feed foreign libs to module-deps as well

anmonteiro20:09:21

to gather their dependencies

anmonteiro20:09:33

but it doesn’t handle the AMD case

dnolen20:09:38

oh you mean it can’t find it

anmonteiro20:09:40

(or rather, that’s the case where ES6 imports AMD)

anmonteiro20:09:56

we don’t feed AMD modules to module_deps

dnolen20:09:28

@anmonteiro can we close this one?

dnolen20:09:32

also should we bump to latest Closure?

anmonteiro21:09:23

@dnolen I think we can close 2289

anmonteiro21:09:34

even though my PRs didn’t make it to this Closure release

anmonteiro21:09:50

but it fixes some other bugs. I’ll put together a patch to bump Closure later