Fork me on GitHub
#cljs-dev
<
2018-03-08
>
dnolen00:03:12

@mfikes there was still a bug

dnolen00:03:03

the concurrent queues don’t fix anything - you simply cannot have threads enter connection and set-connection at the same time

dnolen00:03:08

if two threads can enter then all kinds of bad things can happen

dnolen00:03:13

I tested this, no discernible perf difference, still fast - and I can’t break with my Socket REPL experiments

dnolen00:03:38

anyway, end result - very few changes and browser REPL is now significantly more capable 🙂

richiardiandrea00:03:03

right! so this new lock is effectively doing what the swap! guarantees, sorry if I repeat the obvious, it just makes me feel good to know that I was on the right path 😄 lol a developer needs his/her certainties

mfikes00:03:16

Yeah, perhaps if you wanted to imitate the lock approach you'd have to reach for ref. TBH, though I understand locks better than STM.

bhauman00:03:11

whoever said concurrency is hard is kidding themselves

mfikes00:03:34

Tried master again (with the lock construct) on 2012 mac an repro’d CLJS-2629 after 3508 iterations.

mfikes00:03:22

Oh, hey @bhauman do you happen to know if Figwheel sends Content-Length when talking with the browser? (That might explain why it works…. thinking about CLJS-2629)

bhauman00:03:43

its a websocket so its different

bhauman00:03:53

and http-kit handles it

dnolen01:03:25

@mfikes hrm and it’s the same stacktrace?

dnolen01:03:34

oh 2629, that’s a different problem

dnolen01:03:44

the Safari problem

mfikes01:03:15

Right with 2629, I never had any issues with connections (at least no obvious ones). It is that silly Content-Length thing.

mfikes01:03:42

I finally managed to repro CLJS-2629 on a Late 2013 iMac, after 17000 iterations using the zippy.clj version at https://gist.github.com/mfikes/765ac8e43d70b743e3bfbd02ff4703a1 So perhaps this is getting in the realm where others can repro. (I've now repro'd on 3 separate boxes ranging from 2009 through 2013 vintage)

dnolen01:03:58

@mfikes hrm are we just sending the wrong content length

dnolen01:03:06

in Chrome you can see the iframe requests

mfikes01:03:30

I've definitely tried to ascertain if the content length might be wrong, and I've haven't found evidence of that yet.

dnolen01:03:42

when I eval something, I see Content-Length that’s one less byte then when I copy and paste the response into a file

dnolen01:03:02

@mfikes how are you checking content length?

mfikes01:03:48

I added a bit of code to count the form before calling .getBytes to see if anything there might be going wrong.

mfikes01:03:58

In Safari I can see some evidence of consistency...

mfikes01:03:28

(That's from the Network tab.)

mfikes01:03:52

Crap, sorry we are out of Slack storage space again.

mfikes01:03:26

Anyway, without pasting images, I'm looking in the Network tab which has Size and Transferred numbers.

dnolen01:03:23

@mfikes hrm and when it craps out, no change with the content length?

mfikes01:03:45

Right. As far as I can tell.

dnolen01:03:31

@mfikes and what delay are you using?

mfikes01:03:44

But, what could happen is that we send a Content-Length saying that we the payload is, say 500 bytes, but only send 474, and then Safari will conclude that the network connection must have been "lost"

dnolen01:03:19

but I don’t see how that could hapepn

dnolen01:03:40

oh but zippy.clj has no delay?

mfikes01:03:08

Right, I eliminated the delay, and also the JavaScript console printing, to make it go as fast as possible

dnolen01:03:27

but then I wouldn’t be concerned about this - might just be Safari bug

dnolen01:03:43

I’d only be concerned if this could happen at REPL

mfikes01:03:50

Yeah, it is definitely a corner case thing now.

dnolen01:03:52

or happen programmatically with some frequency

mfikes01:03:05

We know you have to evaluate very quickly, and on very old hardware

dnolen01:03:38

yeah, I really couldn’t repro with this test case I have where I start two connecting pREPL clients in quick succession

dnolen01:03:43

that was crapping out for me before

dnolen01:03:48

nearly always

mfikes01:03:35

I'm still interested in the other, slower, "long-lived" but with substantial delay, style test, to imitate what a real developer might do. (These tight loops are interesting but not the main use case.)

mfikes01:03:42

The only time where rapid evaluation might matter is right at startup, when a lot of stuff is getting evaluated in rapid succession during REPL environment bootstrap—maybe that's where started thinking connections were failing 20% of the time.

dnolen01:03:18

@mfikes hrm when running these tests are you making sure there’s only one browser window open?

dnolen01:03:31

@mfikes going to lower the priority on 2629 for now

mfikes01:03:05

@dnolen Yeah, for tickets like these, it is almost like we could close them as "assessed, but we aren't likely to fix", just so they don't clog up the JIRA

dnolen03:03:02

@mfikes it’s worth keeping these kinds of tickets open, nice history of the various challenges we encounter along the way

dnolen03:03:27

@mfikes is that supposed to be socket REPL, the gif?

mfikes03:03:42

Hah... It took me a while to figure it out 🙂

mfikes03:03:48

Here is my interpretation:

mfikes03:03:39

It represents AOT caching. The idea is that ClojureScript itself is expanding out to embrace the JARs that it pulls from Clojars.

mfikes03:03:49

(Just my interpretation.)

dnolen03:03:50

ah yeah 🙂

dnolen04:03:20

the list is whittling down again - circular dep parallel build and one I’m not really sure what to do about - the Windows one

mfikes04:03:52

One way to look at the Windows path issue https://dev.clojure.org/jira/browse/CLJS-2588 is that it is not a regression introduced by 1.10.x, and it also appears to be a Closure issue perhaps (https://github.com/google/closure-compiler/issues/2611#issuecomment-345595935_

dnolen04:03:42

yeah I’m going to lower the priority

dnolen04:03:43

building another release for people to test

dnolen04:03:01

unless there’s something major I think the last thing I will fix is circular dep check and we should just push this out the door next Monday

dnolen04:03:58

next artifact will be 1.10.145

mfikes04:03:59

Yeah. Documentation can be addressed over the next few days.

mfikes04:03:22

(Cleaning up all the drafts, etc.)

richiardiandrea04:03:43

Will try CLJS-2613 and confirm that it works

richiardiandrea04:03:43

Also I would like to take a stab at applying the shared repl env to the node socket Repl (or more correctly to all socket Repl)

richiardiandrea05:03:18

Or maybe there is no need, will play around

dnolen05:03:22

making it work for node would be great

dnolen05:03:12

Rhino & Nashorn seem lower priority for now

rarous06:03:09

I have regression in npm-deps resolution in 1.10.x for material-components-web. For sub-dependencies it doesn’t resolve the module path correctly. When I require eg. @material/snackbar it fails with Uncaught Error: Undefined nameToPath for module$$material$base$component. Only related issue seems CLJS-2451 but it worked fine in 1.9.946 for me.

rarous09:03:22

Opened new ticket with simplest repro case CLJS-2633

danielcompton09:03:20

I noticed a small bug in https://github.com/clojure/clojurescript/blob/d0be39660f3a65422c3de6a774ceec0b6a863ee2/src/main/clojure/cljs/analyzer.cljc#L3127-L3129

(case ext
          "edn"  (spit cache-file
                   (str (when
                     (str ";; Analyzed by ClojureScript " (util/clojurescript-version) "\n"))
                       (pr-str analysis)))
The extra when is wrong here, if you reindent, you can see it better:
(case ext
          "edn" (spit cache-file
                      (str (when
                             (str ";; Analyzed by ClojureScript " (util/clojurescript-version) "\n"))
                           (pr-str analysis)))
This means that the "Analyzed by ClojureScript 1.9.512" will never show in the cache file header

danielcompton09:03:43

Should I open a ticket, or can someone with commit rights just fix it?

jannis10:03:06

Oh… if I can just convince Lee Byron to add a "browser": "index.js" field to graphql-js and iterall, then nothing needs to be changed in CLJS/CLosure to to support both packages out of the box (for the browser target at least).

jannis10:03:34

That would be good enough for my use case, although probably not a long term solution.

juhoteperi11:03:24

@jannis Which env are you targetting? Iterall at least already says main is index. If you target browser I think Cljs should use browser and then main

juhoteperi11:03:30

for nodejs module then main

jannis11:03:01

@juhoteperi Browser. For that, CLJS picks browser, module, then main. Unfortunately both graphql-js and iterall have an .mjs file in module and no browser entries.

jannis11:03:20

Regardless of the target, the module entries with .mjs will break CLJS and Closure. Easy win could be to convince them to add a browser entry pointing to .js, then at least we could import both packages for the :browser target.

jannis11:03:38

But that’s not really satisfying.

juhoteperi11:03:21

What does that .mjs extension mean? Are they just normal es6 modules?

juhoteperi11:03:36

Does Closure have issue about .mjs support yet?

jannis11:03:30

Ah, Closure.

juhoteperi11:03:59

Or does Closure-compiler need changes in addition to those?

jannis11:03:41

AFAIK, there are is no Closure issue for this yet.

jannis11:03:01

I have Closure modified locally so that it supports .mjs.

jannis11:03:29

But there’s still something missing when resolving dependencies between packages.

jannis11:03:08

Where Closure resolves import ... from "iterall" to module$iterall instead of module$path$to$node_modules$iterall($index).

rarous12:03:53

i think it may be related to https://dev.clojure.org/jira/browse/CLJS-2633. It doesnt use .mjs but it breaks on import ... from ... same way as yours

jannis11:03:51

I’m going back and forth between different approaches because I’m not sure I can get that to work in Closure.

juhoteperi11:03:59

If you open issue at Closure (with or without fixes) it is going to be good idea to provide them commands to test this with just Closure cli, similar to e.g. https://github.com/google/closure-compiler/issues/2634

jannis11:03:20

Yep, that’s good advice

mfikes12:03:18

@danielcompton I’d write a JIRA. It may not be critical because ClojureScript now directly depends on Transit so that branch may be less relevant now. https://github.com/clojure/clojurescript/commit/ff573d12d8f868c980585445ff39b957967b6dce

thheller13:03:05

@jannis FWIW I would stay away from .mjs until the node/webpack world has settled all the issues. currently closure only supports the "strict" version of https://medium.com/webpack/webpack-4-import-and-commonjs-d619d626b655

thheller13:03:02

so it currently breaks if CJS is importing a ES6 that was converted by babel (most of the stuff on npm) since Closure only detects it as CJS and doesn't check the __esModule prop

thheller13:03:07

in shadow-cljs I just ignore module and only use main and browser. too many issues with too many packages each doing weird things with module

jannis13:03:27

Sounds like a good decision.

jannis13:03:27

I’ve been trying to understand and get this to work for over a week, going back and forth all the time. With a "browser": "index.js" added to avoid pickup up the index.mjs, everything worked immediately.

thheller13:03:26

well it also works if you ignore module and just use main

jannis13:03:03

Yep, I haven’t tested that but it should work.

thheller13:03:16

the problem is really that closure doesn't support the "mixed" stuff

jannis13:03:21

Neither CLJS nor Closure are going to pick index.mjs if "main" points to "index".

thheller13:03:24

if everything was pure .mjs it would be fine but it isn't

thheller13:03:30

take this fun example https://unpkg.com/[email protected]/index.es.js it has export { default as AppBar } from './AppBar'; which is https://unpkg.com/[email protected]/AppBar/index.js

thheller13:03:05

so they do have a module index.es.js but that file then includes a CommonJS file.

thheller13:03:17

just because their packaging is bad

thheller13:03:11

thats not the only package on npm doing this weird buggy mix of ES6/CJS and closure doesn't like this at all

thheller13:03:18

for the module$iterall issue you could just rename the filename passed to closure from .mjs to .js

jannis13:03:27

Hm. So what’s the takeaway from this? ClojureScript should ditch looking at "module"?

thheller13:03:27

well until the solution is resolved by the closure compiler that is the solution I have chosen in shadow-cljs yes.

jannis13:03:55

@dnolen ^ What do you think?

thheller13:03:54

.mjs should start getting more adoption now that webpack v4 has support for it although node doesn't event support it without the flag.

thheller13:03:09

then there is also dev:module which should be used during development

thheller13:03:27

the entire situation is pretty messy since the JS world hasn't figured out what to do yet

thheller13:03:35

so everyone is doing something different it seems

mfikes13:03:22

Another unexpected "feature composition synergy": You can use classpath-relative -i and -r to load code and run a REPL (without using -c and -r) and avoid dirtying the disk with out (and also use the ability to use top-level require to avoid the single-segment namespace warn) if running everything from a gist. The Bocko gist illustrates this https://gist.github.com/mfikes/e00202b2de7cc2352fedcf92b1fe60dc

dnolen14:03:38

@jannis we should let @anmonteiro weigh in when he is around

kommen14:03:32

we we’re running cljs 1.10.64 for a week or so, without any problems

kommen14:03:04

just giving 1.10.145 a shot and there seems to be a problem with nodejs targets

kommen14:03:28

getting a whole bunch of

WARNING: Use of undeclared Var cljs.core/not at line 3 /Users/kommen/work/nextjournal.com/journal/src/editor/components/ui/icon.cljs
WARNING: Use of undeclared Var cljs.core/apply at line 3 /Users/kommen/work/nextjournal.com/journal/src/editor/components/ui/icon.cljs
WARNING: Can't take value of macro cljs.core/hash-map at line 3 /Users/kommen/work/nextjournal.com/journal/src/editor/components/ui/icon.cljs

kommen14:03:20

the compiler eventually crashes then with

Caused by: clojure.lang.ExceptionInfo: Can't redefine a constant at line 19 file:/Users/kommen/.m2/repository/org/clojure/clojurescript/1.10.145/clojurescript-1.10.145.jar!/cljs/spec/alpha.cljs {:file "file:/Users/kommen/.m2/repository/org/clojure/clojurescript/1.10.145/clojurescript-1.10.145.jar!/cljs/spec/alpha.cljs", :line 19, :column 1, :tag :cljs/analysis-error}

kommen14:03:44

trying to narrow it down a bit now

dnolen14:03:04

@kommen I can only speak about what the symptoms mean: in the first case cljs.core has not been analyzed, in the second case files are being compiled multiple times

kommen15:03:32

@dnolen both issues go away when I pass :aot-cache false in the compiler options

dnolen15:03:15

@kommen without knowing about what build pipeline you are using I really can’t tell you anything more

dnolen15:03:27

downstream tools maybe doing things that make this not work

kommen15:03:17

@dnolen ok thanks, I’ll see if I can pin it down further

dnolen15:03:42

it would be best if you try to repro your build or part of it with ClojureScript only - so there’s something to compare

dnolen15:03:06

@kommen oh one question - does everything work if not :nodejs target? like your web stuff?

dnolen15:03:48

ok will think about that

kommen15:03:03

@dnolen also, we just build stuff with cljs.build.api, shouldn’t be much which can interfere

kommen15:03:33

will try to build a repro case later

dnolen16:03:49

Hrm I’m wondering if :simple shouldn’t also imply :static-fns and :optimize-constants

mfikes16:03:15

@dnolen Are you thinking of less need to use -co if using -O ?

dnolen17:03:18

I’m just thinking you’d probably just want those on under :simple

dnolen17:03:23

can’t think of a case where you wouldn’t

mfikes17:03:01

These “optimization” defaults change as ClojureScript evolves and their impacts are better understood, so there is no real question about potentially breaking anything I suppose, and it just boils down to “is this the right set of implied defaults”

richiardiandrea17:03:04

just as data point, I always turn both static function and direct invoke on, independently from the optimizations

anmonteiro17:03:09

@dnolen @jannis just now getting to read the backlog

anmonteiro17:03:23

I don’t think we should move away from "module"

anmonteiro17:03:45

as I said yesterday, I’m pretty sure that would make many more packages stop working

anmonteiro17:03:04

there’s an issue that @juhoteperi is unfortunately too familiar with regarding UMD processing in Closure

anmonteiro17:03:48

if we stop taking "module" into account we’re gonna be looking UMD bundles that Closure can’t process

juhoteperi17:03:28

UMD support might be okay currently

juhoteperi17:03:33

better than previously anyway

anmonteiro17:03:38

do you really wanna bet on that? 😛

anmonteiro17:03:03

definitely better than previously, but “okay” is not a word I’d use when different bundlers use different UMD checks

juhoteperi17:03:09

"how many different wrappers there can be..."

anmonteiro17:03:25

famous last words

rauh17:03:14

If I switch the order of -c and --optimizations around it does a build w/o advanced (in clj -m cljs.main --optimizations advanced -c hello-world.core). Bug?

juhoteperi17:03:39

There might be other benefits to using modules instead of UMD bundles also, maybe better DCE?

jannis17:03:15

@anmonteiro Hm, ok. I don’t think I can make Closure work with .mjs entries. I tried but there are still issues with dependencies between ES modules from different packages and I can’t fore the life of me figure out what’s going on. I think I’m going to give up for now if there’s no solution on the horizon.

anmonteiro17:03:38

sorry I can’t look into it either, I’ve been really busy with work the past few months

rauh18:03:28

Also: Doing a :none build and then using --serve gives me an NPE in repl-client-js fn

juhoteperi18:03:42

> it is currently incompatible with running multiple ClojureScript compilers simultaneously on your dev box. Any plans on supporting this? I have quite often maybe 3 or 5 projects open

rauh18:03:11

Another one: Doing (js/process.exit 0) on a node repl with clj -m cljs.main --repl-env node fails a little ugly

richiardiandrea18:03:57

@rauh probably worth Jiras, I will look at the node stuff as soon as I have some time

dnolen18:03:11

-c -m -r must come last, please look at -h

dnolen18:03:30

@mfikes we probably shouldn’t say anything like that in the post

dnolen18:03:15

it’s like Maven - multiple things aren’t really going to cause problems except in the rare case where two things are starting at the same time and you don’t have the thing cached already

dnolen18:03:49

once something is cached, it doesn’t matter how many things go to the cache later

mfikes18:03:12

OK, so drop the paragraph about multiple compilers

dnolen18:03:01

for now, yes - I’d like to see how often this is actually a problem in practice, so far I’ve been running lots of different REPLs, compiling things, and I haven’t encountered cache corruption yet

mfikes18:03:08

Come to think of it, we might already have “corrupt” detection

mfikes18:03:28

Anyway, yea, I’ll zilch that paragraph

dnolen18:03:52

@jannis a fix I would be OK with would be some kind of resolution flag, for your case I wonder if you really need anything more than top level, i.e. what we have is :webpack and we could offer plain :node

jannis18:03:52

What would that flag do? Yes, the package.json -> index.mjs/js is the critical part, but it’s probably not enough to handle it in CLJS, Closure also needs to work. A flag to ditch "module" from the entry fields (which we can also configure in Closure from cljs.closure) would solve it, I think.

anmonteiro18:03:25

^ which is what I had proposed too. but I might like @dnolen’s idea better actually

anmonteiro18:03:44

let’s us abstract over the nitty gritty of Closure if we ever want other resolution modes

anmonteiro18:03:02

i.e. not controlling the specific fields directly but offering a range of predefined options

jannis18:03:23

Ok, so how would this work and what would we have to do for it?

dnolen18:03:17

@jannis not much, our own stuff defines the order

dnolen18:03:26

so look where we have that order and make that pluggable

dnolen18:03:58

this seems like a legitimate :npm-deps regression but unclear where it happened https://dev.clojure.org/jira/browse/CLJS-2633

dnolen18:03:18

if somebody has time to at least sort out the revision where things went wrong that would be a good thing to add to the ticket

dnolen18:03:19

@jannis Closure doesn’t need to work, we imposed our own order, so just let’s make it work from our end - handling .mjs is not under consideration here

jannis18:03:17

@dnolen I don’t understand what you mean by “order” and “pluggable”.

dnolen18:03:39

@jannis the resolution order is hard coded

dnolen18:03:47

don’t hard code it, that’s all I’m saying

dnolen18:03:08

["browser", "module", ...] thing

dnolen18:03:47

far as I can tell that’s all you actually need for your use case

dnolen18:03:53

standard order not :webpack

jannis18:03:18

If ["browser", "main"] is standard order, then yeah, pretty much 🙂

dnolen18:03:29

so this seems like an easy fix

jannis18:03:43

Ok. And you’d like to call this order :node?

dnolen18:03:58

:nodejs I guess for consistency

jannis18:03:20

And the current mode :webpack? Alright, I can definitely implement that! 🙂

dnolen18:03:28

sweet thanks

jannis18:03:32

Thank you 🙂

mfikes18:03:54

I wonder if there are any other high-level (but not marquee) items to list for 1.10.x in here https://github.com/mfikes/clojurescript-site/blob/659a97a2159180cb344f459d147fd57856a0cbae/content/news/2018-xx-yy-release.adoc

mfikes18:03:13

(I’m gonna remove the last thing listed in there about cljs.main)

juhoteperi18:03:36

Closure update is worth mention, the new version includes at least my UMD wrapper fixes and loads of general module processing changes

juhoteperi19:03:41

Hmm, update also fixes requiring ES6 modules from CJS code (or was it the other way...)

anmonteiro19:03:28

@jannis in this case :nodejs would be just ["main"] then

dnolen19:03:17

@mfikes enhanced socket REPL, and also alpha pREPL (requires Clojure 1.10.0-alpha4), I would definitely add @juhoteperi suggestions

jannis19:03:29

How about the flag name? :package-json-resolve-order or is that too specific?

anmonteiro19:03:16

package-json-resolution-type?

anmonteiro19:03:30

or just package-json-resolution?

jannis19:03:26

I like the last suggestion.

juhoteperi19:03:03

@mfikes I added a suggestion for Closure/module processing section to the pull request.

dnolen19:03:38

@anmonteiro @jannis I like the last one too

dnolen19:03:46

@mfikes I would change the order, I think cljs.main, :npm-deps first

dnolen19:03:58

those two have the most impact for day-to-day

dnolen19:03:45

@kommen can you file a bug for your issue - I think that & 2633 need to be fixed (or thoroughly investigated at least before release), critical priority is fine

bhauman20:03:05

@mfikes how did you implement that ClojureScript 1.10.145 header? I'm getting it at every prompt in piggieback

bhauman20:03:26

looks like you are doing it in a different spot than before

bhauman20:03:31

@mfikes yeah, so piggieback is going to be hit by this because it explicitly disables the quit-prompt, but doesn't disable the new title-prompt

bhauman20:03:59

and I would say it is one of the most used CLJS repls for work

bhauman20:03:00

I have a PR in that would prevent this, but there is no telling when it will be accepted

dnolen21:03:24

@mfikes @bhauman let’s just drop the new name for now

mfikes21:03:29

Ahh, that’s an interesting problem.

mfikes21:03:44

It must have an interesting lifecycle.

richiardiandrea21:03:41

I was thinking about this yesterday, probably it should be described with tests.

richiardiandrea21:03:02

so many things to do, so little time

bhauman21:03:08

yeah it creates a dang REPL for every eval

bhauman21:03:53

I have a PR that changes that to an eval instead

dnolen21:03:18

@bhauman hrm too bad we didn’t make REPL envs idempotent before

bhauman21:03:19

trying to get it applied

dnolen21:03:23

that’s the most expensive thing by far

mfikes21:03:32

So it sounds like we can replace the default behavior of quit-prompt to print the version or somesuch

dnolen21:03:57

@mfikes yes just use quit-prompt to print the version, and leave the API alone

bhauman21:03:02

Wait REPL envs are idempotent? BC of new caching?

dnolen21:03:12

@bhauman browser REPL env is now idempotent

mfikes21:03:15

yep, that works, I can put together a patch for tht

dnolen21:03:15

because of Socket REPL

dnolen21:03:27

@bhauman just a little more work for Node.js

bhauman21:03:39

but the REPL itself does an analysis run as well right?

bhauman21:03:04

when it starts?

dnolen21:03:49

I want us to have N number of REPLs attached to the same JS environment

dnolen21:03:00

so this works for browser REPL right now

dnolen21:03:22

@bhauman but it’s also what other REPLs need

dnolen21:03:26

to not reconstruct everything each time

richiardiandrea21:03:20

in an idillic world you could have a browser AND a node AND a nashorn REPL running at the same time 😄

dnolen21:03:52

heh, off topic 🙂

richiardiandrea21:03:55

(on different threads)

bhauman21:03:43

oh this is just for the server.repl s

dnolen21:03:52

it just leverages that browser REPL env is idempotent now

dnolen21:03:06

anybody can do this trick

dnolen21:03:46

so we just need to do the same thing for the others - Node.js being the obvious one to do next

dnolen21:03:53

should have thought of this before when I looked at what piggieback was doing - oh well

dnolen21:03:57

an already setuped repl-env is fast because all of the evaluations are uninteresting

dnolen21:03:08

everything is already required etc.

dnolen21:03:04

0.032s to reuse repl-env 🙂

bhauman21:03:41

so I was finding that medium size code bases got hit by analysis on repl start

dnolen21:03:09

it should be better with new release

dnolen21:03:29

Transit is a dep now

dnolen21:03:43

but yes you will still have to pay then when starting up

dnolen21:03:51

but at least you’re not reading EDN

dnolen21:03:00

it’s at least 4-5X faster

kommen21:03:24

@dnolen re 2639: doesn’t seem to reproduce easily, couldn’t yet build a demo app with nodejs target which shows the same issue as I’m seeing in our project

kommen21:03:26

started ripping out stuff, but it is getting late here though. I do see a different ex info now as before:

kommen21:03:28

Caused by: clojure.lang.ExceptionInfo: Invalid :refer, var cljs.tools.reader.impl.utils/whitespace? does not exist in file file:/Users/kommen/.m2/repository/org/clojure/tools.reader/1.3.0-alpha3/tools.reader-1.3.0-alpha3.jar!/cljs/tools/reader/reader_types.cljs {:tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4739)
	at clojure.core$ex_info.invoke(core.clj:4739)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:697)
	at cljs.analyzer$error.invoke(analyzer.cljc:693)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:695)
	at cljs.analyzer$error.invoke(analyzer.cljc:693)
	at cljs.analyzer$check_uses.invokeStatic(analyzer.cljc:2192)
	at cljs.analyzer$check_uses.invoke(analyzer.cljc:2187)
	at cljs.analyzer$check_use_macros.invokeStatic(analyzer.cljc:2205)
	at cljs.analyzer$check_use_macros.invoke(analyzer.cljc:2195)
	at cljs.analyzer$check_use_macros_inferring_missing$fn__1975.invoke(analyzer.cljc:2217)

kommen21:03:21

maybe this rings a bell with some one…

danielcompton21:03:26

I've noticed that the last modified times on generated JS files are set to match the last modified times of their corresponding CLJS files. This can lead to unexpected caching behaviour: https://github.com/bhauman/lein-figwheel/issues/664

danielcompton21:03:03

Is there a safer/better way to calculate a last modified time that also reflects the input dependencies to a ClojureScript namespace?

danielcompton21:03:33

I was thinking of taking the max date of all last-modified's

dnolen21:03:17

@danielcompton that problem doesn’t seem to have to do anything setting last modified

dnolen21:03:43

it’s only about macro files changes and that not being part of invalidation (because they are not actually part of the build)

dnolen21:03:44

FWIW, I also don’t really think this is very simple to fix, so I don’t know if we should be involved

dnolen21:03:59

you’d have to crawl the dep graph of the macros files to be sure

danielcompton21:03:23

Figwheel sends the last modified time for any served files which is used for browser cache invalidation. I assumed that the last modified time of a JS file would be different after a recompile, but that's not a correct assumption.

danielcompton21:03:47

What's the purpose of setting the last modified time of the generated JS files to match their corresponding CLJS files?

dnolen21:03:08

so you know to recompile

dnolen21:03:15

please leave Figwheel out of this 🙂

dnolen21:03:45

ClojureScript does not watch the file system

dnolen21:03:58

and it’s not going to start doing that

danielcompton21:03:25

I'm not suggesting any of that, I'm saying that the last modified date of compiled JS files isn't able to be reliably used for web server caching headers

dnolen21:03:58

but why does this matter for development?

danielcompton21:03:37

because it's still useful to be able to set caching headers in development to speed up reloads. It's just a confusing behaviour that I wouldn't have expected

danielcompton21:03:01

The answer seems to be that you can't rely on the file modification date for caching CLJS development

danielcompton21:03:50

which is fine, it just wasn't obvious to me

dnolen21:03:56

under local dev nearly all browsers do aggressive caching already

dnolen21:03:15

it’s not my experience this is worthwhile

danielcompton21:03:30

Yeah the caching is too aggressive, which leads to weird behaviour when you're developing

dnolen21:03:48

so you want to do this for more control?

dnolen21:03:19

anyways, your idea needs to be reconciled with figuring out how to know the source is now different from the target

dnolen21:03:31

(I’m listening if you have some idea here)

dnolen21:03:38

but IMO, I never bother with Etags until we go prod

danielcompton21:03:35

Now that I understand what is happening with last modified dates, I think it's safer just to not rely on them for caching purposes, as they aren't always going to reflect the latest changes

danielcompton21:03:48

I'll take a look at the change tracking code, but it seems easier to leave it as is for now

dnolen21:03:24

yes changing how this works doesn’t seem like a good idea at this venture 🙂

danielcompton21:03:10

The compiled JSON/EDN cache files also match the CLJS file modification date, I don't know much about that cache, but does it only cache information from the corresponding cljs file?

dnolen21:03:15

problem is the cache file is optional, so it can’t be used for this

danielcompton21:03:41

I understand, was just wondering if there were other issues lurking based on file modification time

dnolen21:03:58

not that I’m aware of

dnolen22:03:38

the recompilation / reanalysis logic is from 2013