Fork me on GitHub
#cljs-dev
<
2019-06-06
>
lread04:06:25

thanks @mfikes, I notice that there are a couple of PowerShell scripts, I won’t change those, but I can look at getting Windows coverage via git bash for current sh/bash shell scripts.

Roman Liutikov09:06:51

Is it possible to turn on *warn-on-infer* on a project level rather then per file? I’m in the process of adapting a large project for advanced compilation, having externs inference working for entire project would be much more helpful.

Roman Liutikov09:06:43

I think it could be possible to hook into analyzer and selectively set *warn-on-infer* for project namespaces

thheller09:06:08

shadow-cljs does this with :infer-externs :auto but CLJS doesn't support that by default currently

Roman Liutikov09:06:27

@thheller is it possible to hook this code into cljs compiler?

thheller09:06:59

the shadow-cljs code no. doing the same would be easy though.

Roman Liutikov09:06:21

ok, looking into shadow’s sources then, thanks

Roman Liutikov09:06:44

perhaps we could get this into figwheel as well

thheller09:06:43

thats basically the logic. if file is available (ie. not source from a .jar) then assoc :infer-warning true

thheller09:06:11

probably a similar two line patch in CLJS proper

Roman Liutikov12:06:52

@thheller I guess shadow also typehints dotted expressions with ^js to avoid manual hinting? Where is it located in the code?

thheller13:06:44

uhm yeah it does a bunch of stuff regarding externs inference overall to reduce some of the "noise"

Roman Liutikov13:06:48

@thheller Is it a lot of code in shadow? I’m thinking if this would be possible to have on top on cljs compiler as well

thheller13:06:54

well it is implemented entirely differently so you won't get that into CLJS

Roman Liutikov13:06:21

I mean I’d like to have this in a project, not in compiler itself, but as a part of build pipeline.

thheller13:06:09

it is part of the compiler. I decided to simplify and use "untyped" externs inference

thheller13:06:02

so the externs inference in the compiler otherwise is a bit more complicated since it has to propagate type hints more thoroughly

thheller13:06:29

all the code I have for this in shadow-cljs does not do this so it wouldn't work in combination with the stuff in CLJS

thheller13:06:13

but really there are only a couple of points in the compiler that need to be "fixed"

thheller13:06:42

just don't use shadow-cljs as a reference and dig into the actual code in the analyzer a bit

thheller14:06:08

I was just too lazy and wanted something a bit simpler

dnolen14:06:14

other reason for type hinting driven externs inference is for getting that info from Closure libs into ClojureScript

dnolen14:06:23

esp. primitive info - boolean, string, etc.

dnolen14:06:11

relatively minor - but would eliminate another case of manual hinting

thheller14:06:42

yeah my concern was more on the npm integration stuff where type hints won't be available anytime soon

dnolen14:06:08

to be clear externs inference is intended to be generic machinery

dnolen14:06:12

one use case is foreign libs

dnolen14:06:22

the other is deriving information about Closure libs

dnolen14:06:31

docstrings, types, etc.

Roman Liutikov14:06:17

It’s super easy to develop with SIMPLE optimizations, but so hard to switch to ADVANCED afterwards 😞

thheller14:06:40

hmm not really. externs inference is already pretty good. just need to tweak a few cases where it reports false positives or things already in externs

thheller14:06:10

one thing that made a big difference was importing all known properties from the closure compiler and don't warn for those

thheller14:06:52

unfortunately that was in a package-protected method

dnolen14:06:11

@roman01la warn on infer on the whole project would be undesirable at this point and we're not going to do what shadow does for now

thheller14:06:16

but that removed a lot of noise

dnolen14:06:44

I will say developing for a long time w/o advanced is asking for trouble

👍 4
Roman Liutikov14:06:44

@dnolen I understand. I’ve hacked global inference with :warnings option set + a custom warnings handler. At least now I can see something.

dnolen14:06:55

we're not going to solve bad practice problems with workarounds

💯 4
dnolen14:06:24

especially when we spent a ridiculous amount of effort making this stuff better already

thheller14:06:08

well it could use a couple of "fixes" still. IIRC defrecord generates a infer warning

dnolen14:06:45

yeah there's minor stuff, but I haven't run into any show stoppers yet

dnolen14:06:27

nor heard many bad stories lately

thheller14:06:20

well that might be because externs inference isn't very useful when using CLJSJS externs and people just don't enable it

dnolen14:06:43

since plenty of people funnel through the webpack guide

dnolen14:06:55

I haven't worked on a project in over a year that doesn't use webpack in some form

thheller14:06:49

I would expect that it is not uncommon to still have cljsjs/react on the classpath and still end up with its externs

dnolen14:06:37

but anything else you need from npm would go the webpack route and needs externs

dnolen14:06:58

plenty of useful trivial libs that you won't find on CLJSJS

thheller14:06:33

yeah of course. still any wrapper lib that has a CLJSJS package might have externs, even if you don't actually use the lib and just use webpack to get it