Fork me on GitHub
#cljs-dev
<
2020-05-26
>
dazld12:05:08

we're still seeing some inference warnings when using satisfies? - wondering if this is being tracked already? i can try to make a repro case if useful too

mfikes12:05:15

I don't recall any remaining open issues surrounding satisfies? and type inference, so a JIRA with minimal repro would be useful

👍 4
dnolen13:05:14

@roman01la just not interested in changing the string concatenation because of Jsonp api changes, as @thheller alludes the issues are too subtle

dnolen13:05:40

you need to follow Closure conventions here - we're not going to make it more convenient

dnolen13:05:32

we encountered the same thing with the code splitting loader - goog.html.legacyconversions.safeUrlFromString works just fine

bhauman15:05:18

so :npm-deps {package "1.2.3"} is incompatible with :target :bundle ?

bhauman15:05:36

also if folks specify :install-deps the compiler throws an error.

[Figwheel:SEVERE] java.lang.IllegalArgumentException: contains? not supported on type: java.lang.Boolean

dominicm15:05:55

Oh, I thought I'd broken that. ^ I've reproduced that myself.

bhauman15:05:04

because :npm-deps is true

bhauman15:05:20

There just needs to be a guard in the meantime

dnolen16:05:42

:npm-deps means something slightly different now

dnolen16:05:53

it really just declares dependencies

dnolen16:05:48

:install-deps is still there and uses :npm-deps, but it's so slow

dnolen16:05:59

clj -m cljs.main --install-deps preferred

dnolen16:05:22

instead of putting in your build config which slows down build and REPL

dnolen16:05:55

the other thing that :npm-deps does is notify the compiler needs to index node_modules

bhauman16:05:03

There is a bug where merge overwrites :npm-deps with true, and calling :install-deps throws when using the :bundle target

dnolen16:05:59

that's a bit too succinct for 2 bug reports

dnolen16:05:04

is that one thing or two things?

bhauman16:05:55

its really one thing

bhauman16:05:12

:install-deps conflicts with :bundle

bhauman16:05:47

becase add-implicit-deps overwrites :npm-deps with true

bhauman16:05:01

and then install-deps expects a map

dnolen16:05:22

@bhauman fixed in master

bhauman16:05:01

this was an issue that @gdanov debugged this morning

dnolen16:05:21

just tweaked check-npm-deps to handle the boolean case for :npm-deps

dnolen16:05:53

in truth :bundle is just sugar

dnolen16:05:04

you could have encountered this with just compiler options

dnolen16:05:33

i.e. upstream node deps + :install-deps true

bhauman16:05:36

its fair to say that folks should not have deps specified in :npm-deps when wanting a bundle target?

dnolen16:05:38

@bhauman :bundle is specifically designed to work :npm-deps

bhauman16:05:52

to add values to it

dnolen17:05:07

it's designed to work with it period

dnolen17:05:15

you need to be able to declare deps and get upstream deps

dnolen17:05:50

this gets you ClojureScript libs that depend on node_modules instead of CLJSJS etc.

lilactown17:05:01

yeah from what I understood, :bundle means that as a lib author I should def start including :npm-deps in a deps.cljs in my lib

lilactown17:05:45

if there’s something broken with :npm-deps and :bundle then I’d assume it’s a bug

dnolen17:05:27

Krell depends on this feature, and I believe some demos/examples have been made

bhauman17:05:19

OK my main question is: Is it intended that :bundle will always set :npm-deps to true, even if its populated with deps declarations?

dnolen17:05:41

oh that's a bug if it overwrite

bhauman17:05:01

yes that’s what I was getting at 🙂

bhauman17:05:28

in this clause it gets overwritten

bhauman17:05:05

I’m super close to releasing the next version of figwheel that supports all of this better and I just wanted to clarify things

dnolen17:05:20

@bhauman yeah I didn't notice it because I stopped using :install-deps true

dnolen17:05:23

fixing

👍 4
dnolen17:05:59

yeah definitely two different bugs, both fixed now @bhauman

👍 8
bhauman17:05:39

those were tough ones for folks switching over simple_smile

dnolen17:05:52

yeah pretty bad bugs, let me know when you want me to cut another release to depend on

bhauman18:05:53

yeah a new release pretty soon would be great

dnolen18:05:36

started one now

dominicm18:05:34

@dnolen I know I asked before, but if I wanted to programmatically install deps, is there another api I should use? Probably the same one as cljs.main?

dnolen18:05:24

yes same as cljs.main