Clojurians
#cljs-dev
<
2017-07-14
>

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

dnolen00:07:07

ah we do have a little bit more work to do wrt. all this modules I think - REPL support

dnolen00:07:19

I’ve updated the Blocker/Critical list to reflect

dnolen00:07:24

will spend time on this tomorrow

anmonteiro00:07:28

can you post the links to those again?

dnolen00:07:31

(unless somebody nabs first)

anmonteiro00:07:51

no I mean, the lists

mfikes00:07:12

Maybe in the future: A diagnostic for non-core code using js*

dnolen00:07:51

@mfikes it’s possible but I don’t actually care that much about that one

dnolen00:07:57

there are no rules, and nothing to impose

dnolen00:07:16

a warning would be nice perhaps for people who want to lint

anmonteiro00:07:23

don’t really understand CLJS-1801

mfikes00:07:24

Right, it is unlikely to change

dnolen00:07:39

@anmonteiro let me add a description it may no longer be valid

darwin00:07:44

IMO there may be good reasons to use js* for people who know what they are doing :slightly_smiling_face:

darwin00:07:25

btw. canary just generated first downloadable jar release: https://github.com/cljs-oss/canary/releases/tag/r1.9.769-33e8067

dnolen00:07:10

@anmonteiro updated - sadly I didn’t record my thoughts when I opened it and it’s pretty old

dnolen00:07:22

gotta run for now

dnolen01:07:40

@mfikes https://dev.clojure.org/jira/browse/CLJS-2231 I don’t think this is a problem in master, we check to see if the fn is set

mfikes01:07:00

Well, it is checked for truthyness

mfikes01:07:47

(The default is a fn that throws.)

dnolen01:07:29

@mfikes I think we should stop setting them to fns that throw and push that logic one level up

dnolen01:07:45

i.e. move the throwing to string-print

mfikes01:07:09

I’ll try that and see how things behave.

dnolen01:07:10

updated the ticket with desired change

dnolen01:07:13

moving throw to string-print is fine

juhoteperi09:07:27

Do we have other examples for :preprocess than JSX?

juhoteperi09:07:05

I thought quickly about TypeScript, but I think that needs proper compilation instead of per file preprocessing.

juhoteperi09:07:22

Hmh, where did the message go :smile:

juhoteperi09:07:32

Yes, Closure does have some kind of support for TypeScript.

juhoteperi09:07:13

:commonjs/`:amd` processing uses compiler options :language-in to process the sources, :es6 processing uses always :es2016

juhoteperi09:07:49

Perhaps js-modules should allow setting :language-in per module-type, or directory

juhoteperi10:07:37

Hmh, or at least Closure supports "TypeScript-style type annotations: > Initial work on supporting TypeScript-style type annotations, with the --language_in=ES6_TYPED flag.

juhoteperi10:07:12

That note from 2015 is only thing in Closure changelog about TS, so I'm not sure how good the support is

juhoteperi10:07:21

I updated the preprocess post with a bit more content: https://github.com/clojure/clojurescript-site/pull/112

juhoteperi10:07:08

The headers can be probably improved, but I think the order of content should be okay

dnolen10:07:59

@juhoteperi funny enough I was thinking about that exact case yesterday - I think we should hold off on how we will bulk preprocess - I don’t really think anything we’ve decided closes any interesting doors

juhoteperi10:07:23

Yeah, if we need to later change how preprocessing works, we can for example group-by the sources similar to module processing. And this doesn't break the :preprocess option.

dnolen10:07:58

@juhoteperi yeah also there’s a lot of new stuff here and I think further changes should probably be guided by usage feedback

mfikes11:07:03

I’m working through an issue where it appears a warn is printed for a bad aget or aset and this is potentially in the communication infrastructure between Figwheel and the app, and the warn itself seems to cause yet another, and it cascades. I end up seeing this in the Figwheel console, but no warnings there

Figwheel: message from client couldn't be read!
Figwheel: message from client couldn't be read!
Figwheel: message from client couldn't be read!
...

mfikes12:07:15

^ Sorted the above. Will send a PR to Figwheel. Also, less critical but cool: The new feature is pointing out invalid uses of aget at runtime in core.async. A simple example is here where aget is called, and then the result is checked for nil rather than also first checking to see if the array is non-empty: https://github.com/clojure/core.async/blob/master/src/main/clojure/cljs/core/async/impl/timers.cljs#L84

dnolen12:07:32

@mfikes nice

mfikes12:07:46

Good thing: Nothing wrong with the ClojureScript compiler so far with my tests.

dnolen12:07:36

that good news :slightly_smiling_face:

dnolen12:07:32

testing things at the REPL is good thing to focus on. Going to start digging into requiring all the different supported module types at the REPL myself.

dnolen12:07:29

watching all these single function lodash modules get copied over during testing :unamused:

dnolen12:07:59

just refactored resolve-var to clean up the module stuff like I preferred https://github.com/clojure/clojurescript/commit/4e85b044902b5a32a0dcc6e1e5f4ff22bbaf0505

dnolen12:07:34

lein test and script/test passing but could definitely use more testing - and if anything fails - more test cases

dnolen12:07:22

further cleanup is possible if we unify JS module indexing instead of doing it separately as we are now

mfikes12:07:36

There might be a useful change to consider for the aget runtime logging. I think we are only getting the message and not the stack. I suspect this is due to str being applied to the exception.

mfikes12:07:22

I’ll try such a change locally and see what I get. I have bad aget somewhere occuring at runtime, but all I’m seeing is the "Error: Assert failed: (or (array? array) (js/goog.isArrayLike array))" portion, making it hard to track down.

darwin12:07:23

@mfikes maybe unrelated, but in cljs-oops in dev-mode, I did extra work to make sure that assertions are properly reported pointing to user code at call site location - not somewhere into my library code

mfikes12:07:26

Yes. In our case, I think we may have the call to checked-aget at the top of the stack, and the rest of the stack being user code. (It is really just using assert)

darwin12:07:02

for example in devtools console user will see link to your checked-aget, which is not that useful

darwin12:07:24

if you don’t also print full callstack (which is verbose)

mfikes12:07:53

Ahh, right. A challenge too is this is all after macroexpansion. The line number appear to point back to the macro call site.

darwin12:07:53

cljs-oops is also just a macro expansion, the trick is to generate js/Error. object at the call site and then throw it from within you lib code

darwin12:07:25

but this might not be appropriate for cljs.core, just wanted to mention this technique

dnolen12:07:34

@mfikes where is that happening? re str on checked array error, I thought we call *print-err-fn* directly?

dnolen12:07:43

I tested this in the browser and I saw reasonable traces

dnolen12:07:30

(meaning I saw the caller)

mfikes12:07:43

I haven’t tracked it down. This might be something somewhere in the RN stack

dnolen13:07:06

yeah I’m pretty sure what’s being done is entirely reasonable for both Node.js & Browser

mfikes13:07:18

@dnolen When you tested, were you using Figwheel?

dnolen13:07:39

no, I’m not doing any testing with anything else and I don’t really intend to

dnolen13:07:47

I’m focusing on the fundamental surface area

dnolen13:07:59

(which is already huge)

mfikes13:07:56

That makes sense. I’m digging into Figwheel code and I see stuff that explains it I believe. Not a compiler problem. It seems to let the console log it, which would be RN, and then it attempts to serialize the exception back, which leads to issues:

ios:cljs.user=> (*print-err-fn* *e)
Figwheel: message from client couldn't be read!
You are right, you could spend all day figuring out non-compiler stuff.

dnolen13:07:20

@mfikes I just mean there’s a ton for me to QA with the base REPLs + the modules work

dnolen13:07:27

getting it to work will make it work downstream

mfikes14:07:35

I did one more careful read of the “Checked Array Access” sneak preview post and found one small error (it referred to unsafe-get instead of unchecked-get). Pushed a fix for that. Otherwise it LGTM.

dnolen14:07:56

@mfikes let me read over it one more time

mfikes14:07:10

Feel free to send nits to fix

dnolen14:07:27

@mfikes I submitted a bunch of small edits

dnolen14:07:39

mostly to provide slightly clearer narrative in the 1st half

mfikes14:07:46

Cool. Nearly finished updating them

mfikes14:07:06

@dnolen Updated. I took a stab at interpreting the change you had in mind surrounding the “Readers familiar with JavaScript will note…” revision.

mfikes14:07:15

(The other ones were pretty clear.)

dnolen14:07:27

I added a commit nit too

mfikes14:07:25

Ahh, I see there are a couple of further changes I missed right near the end. What was the commit nit?

dnolen14:07:43

er commit -> comma

dnolen14:07:07

@mfikes there are 2 new comments on the latest PR

mfikes14:07:17

Ahh, add two comments. Got it.

mfikes14:07:22

Commas :slightly_smiling_face:

mfikes14:07:51

Two commas have been added. I’m reading it again through to QA my changes.

mfikes14:07:01

I suppose with the removal of the change to js* to support parameters, “Again, the theme of successive refinement.” odd (the “again” part.)

dnolen15:07:45

reading over that again now

dnolen15:07:23

@mfikes agreed I would drop, I think the theme of refinement is clear just in the flow of the prose

mfikes15:07:47

Cool. I slipped it in with one word: “As time moved on, our friend aget (as well as aset) was refined to”

dnolen15:07:26

also pluralize point is right

dnolen15:07:33

alternatives

mfikes15:07:41

Cool. I see no other errors at this point.

dnolen15:07:47

@mfikes I say ship it

mfikes15:07:05

Me too. Go for launch of 3rd post

mfikes15:07:36

It is appearing now. Taking a look…

mfikes15:07:23

I don’t see any formatting errors :+1:

shaun-mahood15:07:38

Not sure if that will actually work (first time trying to post a picture in a thread), but it's the only thing in the post that seemed like an issue to me - I suspect it can be fixed by just removing the extra aget after the snippet.

shaun-mahood15:07:50

Great post though, hopefully it will help reduce the questions. It does make me wonder how many people will ask for oget and oset sugar to wrap the goog.object calls

shaun-mahood15:07:43

Well, that would have been a good idea to check first :slightly_smiling_face:

mfikes16:07:57

Something recently put on master is causing something downstream in my RN project to complain about module-deps. Will bisect. It is not causing any behavior to break AFAICT. https://gist.github.com/mfikes/be782a414965cb3d2b2637c1ed29d54a

mfikes16:07:53

Another aside: Figwheel hot-load updates after changing files used to take 2 seconds for me with the 1.9.671 release but now take about 12 seconds with master (with :checked-arrays off). Sorry for sharing without a minimal repro.

mfikes16:07:42

The module-deps observation above is with the very last commit on master ((https://github.com/clojure/clojurescript/commit/2ba8ec76ec3977143afc5137e22c47838ac9d5a7) but not the prior.

mfikes16:07:40

I’m bisecting to find the 2 s -> 12 s slowdown

dnolen16:07:52

@mfikes we do index the top level of node_modules though that would surprise me if that was a problem

dnolen16:07:55

@mfikes great!

dnolen16:07:10

if you find a clue will divert some time today to look at it

mfikes16:07:23

Yeah, I again apologize for no minimal repro. Willing to try ideas locally if any are had.

mfikes16:07:37

I’ll also try to find out more.

dnolen16:07:27

@mfikes do you have :verbose on? if so you’d see if base node deps are getting installed in the case of your gist

mfikes16:07:53

I don’t have :verbose on. Will try that after the 2 -> 12 bisect

dnolen16:07:55

if you do then somehow Figwheel is skipping that step, and your error would make sense

anmonteiro16:07:29

@mfikes the 2->12 problem might be related to us reindexing the node_modules directory on every file save?

mfikes16:07:00

I have a few more steps to go on the bisect. Will let you know.

mfikes17:07:30

@anmonteiro I suspect you are correct. The 2->12 problem: https://dev.clojure.org/jira/browse/CLJS-2238

mfikes17:07:03

@dnolen Negative result on :verbose shedding any light. It doesn’t appear to log anything relevant: https://gist.github.com/mfikes/8a4d210657fc8baa3064c869d5646520

mfikes17:07:51

I’ll dig locally to see if I can find something minimal

dnolen18:07:46

Might need to look at how Figwheel calls build?

mfikes18:07:29

One thing I’m wondering: Just because I have a node_modules and package.json file sitting in my current directory, does that mean that ClojureScript should assume I’m using it? (I suspect it is just there for the React Native packager to have fun with.)

mfikes18:07:27

(This is a bit of ignorance on my part—I didn’t build re-natal)

dnolen18:07:59

I think probably (node_modules) but should probably only index once, memoized like classpath libs.

mfikes18:07:25

It’s probably OK. I added some util/debug-prns to look at the sets involved in handle-js-modules: The requires set is stuff I recognize, like ClojureScript and some goog namespaces, taking maybe 2 inches to print in my console. The top-level set takes approximately 1/3 of a mile of console to print, and their intersection (the important bit, node-required), is emtpy. So it doesn’t seem to functionally hurt, and the perf would be fixed with good memoization.

mfikes18:07:23

In terms of counts:

requires 77
top-level 14748
node-required 0
That second top-level number prints about 12 seconds after the first.

anmonteiro18:07:26

I agree we should memoize it somehow

anmonteiro18:07:37

might be hard to do because the indexing code actually has side-effects

anmonteiro18:07:56

actually there’s pure code in there

anmonteiro18:07:27

@mfikes let me put together a tentative patch that you can try

dnolen18:07:36

@anmonteiro I was just about to commit something unless you’re already there

anmonteiro18:07:53

I was gonna start working on it right now

dnolen18:07:00

I was just going to name index-node-modules-dir -> index-node-modules-dir*

dnolen18:07:09

and then (def index-node-modules-dir (memoize ...))

anmonteiro18:07:11

separating the getting where we get the file-seq, to the part where we transform

dnolen18:07:17

same as the class path stuff

anmonteiro18:07:21

@dnolen I don’t think that’s enough

anmonteiro18:07:28

because the contents of node_modules should change

anmonteiro18:07:37

and we could handle that gracefully

dnolen18:07:39

right I don’t really care about that case

dnolen18:07:45

we don’t handle that for classpath libs

dnolen18:07:58

so I don’t know why we have to change expectations here

anmonteiro18:07:59

I’m pretty sure it doesn’t take long to get the fileseq

anmonteiro18:07:18

little trouble for good benefit

dnolen18:07:26

but not really

dnolen18:07:36

since it’s different expectations for different things

dnolen18:07:57

and that would have to be explained - better for it just be the same way

anmonteiro18:07:00

hrm so should I put something together for you to look at or don’t you see any value?

dnolen18:07:22

I’m just asking what your answer is for the fact that it will be different

dnolen18:07:31

and for very non-obvious reasons for many users

anmonteiro18:07:47

I guess we shouldn’t memoize impure code :slightly_smiling_face:

anmonteiro18:07:02

might be a reason you don’t care about

dnolen18:07:21

I’m also ok with making it work, but then it’s nothing to point out

anmonteiro18:07:34

cool let me put something together

anmonteiro18:07:50

if we find it still has a significant perf cost we can simplify it to your approach

mfikes18:07:04

As an aside, I’m now pretty sure the module-deps stuff that gets logged (https://gist.github.com/mfikes/be782a414965cb3d2b2637c1ed29d54a) is somehow related, as it does 3 of them, each 12 seconds apart from each other

dnolen18:07:24

@anmonteiro let me do the first thing and have @mfikes test that

dnolen18:07:33

and then you can work off that for your patch

anmonteiro18:07:46

@dnolen there’s also another problem here as @mfikes pointed out

anmonteiro18:07:58

we’re shelling out to Node even if :npm-deps is not in the compiler options

anmonteiro18:07:04

in handle-js-modules

anmonteiro18:07:06

@dnolen I have a fix ready for CLJS-2238 using the approach I suggested

anmonteiro18:07:18

I was seeing this problem while developing Lumo too and this fix + CLJS-2240 solves it

anmonteiro18:07:26

reloads are instant now

anmonteiro18:07:43

let me attach the patch to CLJS-2238

anmonteiro18:07:15

@dnolen patch attached to CLJS-2238

anmonteiro18:07:01

@mfikes can you apply CLJS-2238 on top of current master and report back?

dnolen18:07:10

@anmonteiro oh yeah, ok that’s better, you’re just memoizing the provide computing

dnolen18:07:17

crawling the file seq isn’t actually slow

dnolen19:07:36

just pushed that to master

dnolen19:07:07

@anmonteiro I’m assuming nothing cropped up around my resolve-var refactor

anmonteiro19:07:21

I looked over it and looks good

anmonteiro19:07:33

tried it and didn’t see any weirdness

anmonteiro19:07:53

@dnolen CI has also been green which is reassuring https://github.com/mfikes/clojurescript/commits/master

anmonteiro19:07:10

we have tests that cover the resolve-var thing

mfikes19:07:14

@anmonteiro Thanks for being kind to my 5-yo computer’s CPUs :slightly_smiling_face:

anmonteiro19:07:36

I’ll take that as the patch is working :slightly_smiling_face:

mfikes19:07:03

:athletic_shoe:

mfikes19:07:12

Yes it is very fast now

anmonteiro19:07:40

great. I’m glad you brought it up, I had noticed it yesterday but failed to open a ticket

dnolen19:07:09

just got (require '["lodash/array" :as lodash-array]) working in the standard Node.js REPL

dnolen19:07:34

about to try that with browser REPL

dnolen19:07:22

@anmonteiro hrm, looks like there might be something wrong with maybe-install-npm-deps!?

dnolen19:07:36

when I try with an uberjar doesn’t seem to create node_modules

dnolen19:07:56

because of missing package.json

anmonteiro20:07:08

@dnolen I think if you can upgrade to NPM 5 that won’t be an issue

dnolen20:07:27

@anmonteiro do you see a problem with just creating it temporarily if it doesn’t exist?

dnolen20:07:34

that’s what I was about to do

dnolen20:07:41

it’s a real usability bummer otherwise

anmonteiro20:07:01

if it doesn’t exist we could just output one that has {}

anmonteiro20:07:12

not sure if we should cleanup after ourselves or not

dnolen20:07:19

yeah that’s what I’m thinking

dnolen20:07:25

I think just creating it is fine now that you mention it

dnolen20:07:00

@anmonteiro hrm I don’t really under what your gen-user-ns stuff does when it’s handed a form

dnolen20:07:09

first glance seems like it couldn’t work at all

anmonteiro20:07:31

I think we made it work for some Figwheel usecase

dnolen20:07:16

(gen-user-ns '(require '["lodash/array" :as lodash-array]))

dnolen20:07:28

right but the comment doesn’t make any sense

anmonteiro20:07:30

regarding that case

dnolen20:07:31

since it just doesn’t work

anmonteiro20:07:34

:slightly_smiling_face:

anmonteiro20:07:54

I don’t really remember why that’s there

dnolen20:07:16

well which part? :slightly_smiling_face:

dnolen20:07:02

I think what’s there assumes src is a string naming a file

dnolen20:07:35

so I think gen-user-ns needs to handle sequential? case

anmonteiro20:07:30

why do we need that case?

anmonteiro20:07:36

out of curiosity

dnolen20:07:29

because require forms at the REPL

dnolen20:07:25

in order to handle modules correctly we need to convert require into ijs by calling parse-ns on result ns* form

dnolen20:07:17

and then call handle-js-modules with that

dnolen20:07:27

this has to be done before emit on the AST

dnolen20:07:38

because if you don’t do that, all the load-lib stuff breaks

dnolen20:07:23

this is all simpler in Lumo for the obvious reasons :slightly_smiling_face:

anmonteiro20:07:54

yeah I get it

dnolen20:07:31

Node.js REPL + node_modules is already working w/ Uberjar BTW - it’s sweet

dnolen20:07:43

this bit is for browser REPL, but of course everyone downstream wins for free

dnolen21:07:52

so close …

dnolen21:07:27

@anmonteiro another question what was the rationale for the alias duplicate check

dnolen21:07:37

some of these things are really annoying since they prioritize compilation over REPL

dnolen21:07:44

like the constant in file check

anmonteiro21:07:45

the alias duplicate check was actually meant for the REPL

anmonteiro21:07:11

where you shouldn’t be able to require something with an alias that is already aliasing another namespace

anmonteiro21:07:19

Clojure does it and I thought we should do that too\

anmonteiro21:07:34

maybe there’s a bug if you’re getting that error?

anmonteiro21:07:51

or are you trying to use the same alias for one that already exists?

dnolen21:07:08

@anmonteiro I think maybe you didn’t check for harmless case?

dnolen21:07:12

the exact same alias twice?

anmonteiro21:07:07

@dnolen

ClojureScript Node.js REPL server listening on 50913
To quit, type: :cljs/quit
cljs.user=> (require '[clojure.string :as s])
nil
cljs.user=> (require '[clojure.string :as s])
nil
cljs.user=>

anmonteiro21:07:22

looks like I did

anmonteiro21:07:37

but then if I do (require '[clojure.spec.alpha :as s]) it’ll throw an error

anmonteiro21:07:57

I’m sure I specifically checked for that case

anmonteiro21:07:38

this should also be idempotent, across several analysis passes

dnolen21:07:27

the problem here is pretty circuitous, one second

dnolen21:07:17

@anmonteiro here’s the problem, I’m thinking about disabling that thing and if you want to figure out a way to bring it back you can

dnolen21:07:23

short description

dnolen21:07:32

at the REPL you don’t know anything - there are no source files

dnolen21:07:45

there are no inputs, there are no known requires, no known provides

dnolen21:07:04

in order to even know that somebody wants to loads a module you must process the AST at least twice

dnolen21:07:17

first time to figure out that you have ns* or ns

dnolen21:07:33

(but this is a side effect, you’re going to record incorrect information)

dnolen21:07:52

we then preprocess JS modules which updates everything to the real names

dnolen21:07:03

we need to parse the ns form again to get the REAL one

dnolen21:07:08

but the dupe check fires

dnolen21:07:10

there’s lot of gross ways to deal with this but I’m not really interested

dnolen21:07:24

one option would be to suppress the dupe check the second time via dynamic var

dnolen21:07:43

Maybe will go with that and cleanup later

anmonteiro21:07:00

@dnolen so you’re saying that we may have 2 requires (2 passes, really) that are really trying to load the same namespace but it gets different names?

dnolen21:07:47

@anmonteiro it gets different names because don’t know what name Google Closure is going to invent during process-js-modules

anmonteiro21:07:27

that makes total sense

dnolen21:07:29

this is already true in build compiles - but it’s less of deal there because process-js-modules happens before you compile anything else

dnolen22:07:13

still more work to do though :stuck_out_tongue:

anmonteiro22:07:06

^ @dnolen this looks like you’re processing a file through Closure under Node.js ?

dnolen22:07:31

no browser REPL

anmonteiro22:07:44

when really if you have :npm-deps + :target :nodejs it should just emit a js/require

anmonteiro22:07:50

module$Users$davidnolen$development$clojure$hello-world$node-modules$lodash$array

anmonteiro22:07:59

^ this is a name that Closure would give

anmonteiro22:07:21

we use namespace-name$node$module$lodash_SLASH_array

anmonteiro22:07:53

without any investigation I would say that somewhere we’re not binding :target :nodejs :slightly_smiling_face:

anmonteiro22:07:46

e.g. in (:options @env/*compiler*)

dnolen22:07:02

@anmonteiro you’re missing what I’m saying

anmonteiro22:07:04

I can look closer in 1 hour or so

dnolen22:07:09

this is browser REPL :slightly_smiling_face:

dnolen22:07:14

there is no Node.js here

anmonteiro22:07:22

no browser REPL

anmonteiro22:07:28

missed a comma there :stuck_out_tongue:

dnolen22:07:44

this is working and it’s pretty amazing - just need to sort out the corner cases

dnolen22:07:47

Node.js REPL is done

anmonteiro22:07:12

oh yeah I should have read the minimal repro

dnolen22:07:26

I mean I think we could argue it’s easier to use Node modules interactively from ClojureScript in a browser than it is in JavaScript

dnolen22:07:32

also all this stuff is hot-loading incremental compile

dnolen22:07:48

triggered by REPL interactions

shaun-mahood22:07:03

Pretty sure it's been said before, but you are all killing it lately - amazingly inspiring to see this much work and love go into ClojureScript!

dnolen22:07:09

aaand Node.js require in browser is idempotent

anmonteiro22:07:22

oh you figured out CLJS-2241?

dnolen22:07:06

this is blowing my mind :slightly_smiling_face:

anmonteiro22:07:31

I don’t understand your comment though

anmonteiro22:07:37

> need new AST after we know what the modules are

anmonteiro22:07:51

but seems like the commit is removing 1 analyze pass

anmonteiro22:07:41

@dnolen I wonder if this fix breaks something else

anmonteiro22:07:49

I think that 2nd analysis was there for a reason

danielcompton22:07:31

Did anyone else get any emails from Kyle Pennell at Stackshare about the CircleCI article? I got an automated one from having contributed to CLJS

anmonteiro22:07:03

got one for Om too

richiardiandrea22:07:45

Got one too, very spammy I guess

darwin22:07:22

it is pretty clever, to scrape repo contributors and then get their emails from github web or git commits

dnolen22:07:53

@anmonteiro I don’t think so, pretty sure vestigial as well

anmonteiro22:07:09

cool. I don’t understand the fix though

anmonteiro22:07:21

how does removing that analyze call fix the issue?

dnolen23:07:00

@anmonteiro we didn’t need that one it was suppressing a bunch of stuff

dnolen23:07:13

no-warn, no analyze deps, etc.

anmonteiro23:07:08

and it solely fixed the issue?

dnolen23:07:38

@anmonteiro yeah

dnolen23:07:14

@anmonteiro it’s was trying to analyze the transformed module (which isn’t possible currently)

dnolen23:07:20

I agree it might make sense to make that work

dnolen23:07:32

but I’m trying to get through the big problems first

anmonteiro23:07:00

you’re just leaving me with more questions :slightly_smiling_face:

anmonteiro23:07:04

not that it’s a bad thing

anmonteiro23:07:14

but what do you mean by analyzing the transformed module?

anmonteiro23:07:22

the closure namespace?

anmonteiro23:07:14

Loving the fact that function bodies are not printed now btw

dnolen23:07:39

@anmonteiro yes the processed closure namespace

anmonteiro23:07:50

is the fact that they’re printed as #object[add] to maintain compat with Clojure?

dnolen23:07:47

more or less

dnolen23:07:52

gotta run, bbiab