Fork me on GitHub
#cljs-dev
<
2020-08-14
>
plexus08:08:30

still something not quite right with :npm-deps and :install-deps. What would be the intended behavior if :npm-deps false :install-deps true? install only upstream, or install nothing?

thheller10:08:56

I thought :install-deps is gone and instead now a separate command? cljs.main --install-deps or so?

plexus10:08:22

it's still there https://clojurescript.org/reference/compiler-options#install-deps , should it be marked as deprecated in the docs?

thheller10:08:27

hmm no clue. maybe both still work.

dnolen12:08:57

@plexus install nothing

dnolen12:08:19

erm actually no

dnolen12:08:12

@plexus I would say you have to install upstream because otherwise your dependencies aren't going to work

plexus12:08:42

ok, so :npm-deps false should be treated the same as :npm-deps nil?

dnolen12:08:21

yeah I don't think :npm-deps false-y is that meaningful anymore

plexus12:08:13

the docs say it defaults to false but I believe it defaults to nil / absent so I guess that's really the same thing

dnolen12:08:53

fortunately the docs aren't overly specific

plexus12:08:02

whereas :npm-deps {} == :npm-deps true? i.e. use the node_modules that is there

dnolen12:08:09

I think we should just edit it to clarify that it doesn't affect upstream deps

plexus12:08:29

yeah those docs could definitely use some clarification... I'll have a look at that

dnolen12:08:40

I think once we allowed upstream deps

dnolen12:08:57

:npm-deps being a global control over that stopped being desirable

dnolen12:08:21

again I think the docs aren't particularly clear about the behavior so I don't think we're painted into a corner

plexus12:08:23

the thing is that if false = nil and true = {}, then we can normalize :npm-deps that way, so we don't have to check for nil/boolean/map everywhere it's used

dnolen12:08:28

sure, that would remove unnecessary complexity from the code, happy to see that done

dnolen18:08:08

@dpsutton I finally got to take a look at 3620, sorry for delay that's one a bit of brain twister so I kept putting it off

dpsutton18:08:40

no worries. i understand how things go in spurts. i am in no way demanding your time šŸ™‚

dnolen18:08:43

the first question I have is whether you considered not tagging and detecting whether you effectively have an and or or

dnolen18:08:15

not a trick question - just wondering if you thought about it

dpsutton18:08:07

i understand. and no i did not think about that approach

dpsutton18:08:14

i suppose that would benefit by making this optimization more general

dnolen18:08:05

yeah I'm think we should do it that way

dnolen18:08:25

it's not as hard as it sounds I think

dnolen18:08:35

because and / or always look the same

dnolen18:08:20

let* a value which much be a simple type - and if

dnolen18:08:47

because passes go inside out

dnolen18:08:03

you don't have to do much, either you can fold in the children into the test or you can't

dnolen18:08:41

in the case of and you'll find the thing you can fold in the true branch

dnolen18:08:19

in the case of or, in the else branch

dnolen18:08:59

(if and# and# and#) is actually a terminal case

dnolen18:08:09

you can collapse it to and#

dnolen18:08:16

but then the parent can collapse that

dnolen18:08:22

and then the parent can collapse that, etc.

dpsutton18:08:44

been a bit since i looked at it. are the ast passes a prewalk or postwalk?

dnolen18:08:13

pretty sure postwalk, so this would work

dnolen19:08:05

@dpsutton also if you're busy now that I've spent some time thinking about it I could give it a go over the weekend

dnolen19:08:32

but also happy to let you take it!

dpsutton19:08:07

Iā€™m on vacation so give it a crack.

dpsutton19:08:32

If you have an itch to livestream or record it I think that would be amazing. Big ask though :)

dnolen19:08:53

not a bad idea