Fork me on GitHub
#cljs-dev
<
2017-10-10
>
mhuebert16:10:34

some feedback from using the new JS-deps stuff in the wild: while very cool to be able to bring things in from npm, I’m finding it difficult to know where my JS deps come from. If I put a :foreign-lib in deps.clj that maps some namespaces to a javascript file, I don’t know whether this will be overridden by something in node_modules, or another foreign dep from cljsjs (which I include to provide externs). This leads to scenarios where sometimes I can “fix” my build by running npm uninstall <the-lib>, which feels wrong. It’s possible that I have been using :foreign-libs badly all along, but I thought I was getting the hang of this.

dnolen17:10:47

it’s possible we could do more reporting here

dnolen17:10:55

but neither feature was ever intended to be used willy nilly

dnolen17:10:10

you should always know what’s in your dep graph, we’re not going to magically solve that probelm

juhoteperi17:10:44

It might also make sense (be possible) to have a warning if foreign-libs depend on node modules (or other way around), as mixing two probably doesn't work

mhuebert17:10:20

To me it is a bit of 'magic' that a foreign lib which previously did one thing will do another if there happens to be something in node_modules with a particular name. There does not appear to be explicit configuration of where these deps come from?

juhoteperi18:10:10

foreign libs didn't previously expose libs with the same name as node modules use

juhoteperi18:10:17

and the foreign lib doesn't change, it is the require in Cljs code which works differently, refering to node module instead of foreign lib

juhoteperi18:10:23

but yeah, this is a bit magical

dnolen18:10:49

@mhuebert it’s a tradeoff, you might be managing stuff under node_modules yourself

dnolen18:10:04

I don’t actually understand what problem you are trying to describe since you aren’t really giving it a name

dnolen18:10:23

all I’m hearing so far as is that you understand that clashes are possible

dnolen18:10:27

we could report those

dnolen18:10:56

this isn’t really a new problem - since lib lookup is based on the classpath already

dnolen18:10:03

it’s just that JVM deps tend to namespace

dnolen18:10:06

JS stuff not so much

dnolen18:10:33

and clashes are more likely since we look at two places

dnolen18:10:06

classpath, and node_modules and the later is generally less disciplined as result of users

mhuebert18:10:38

I think reporting clashes would be very helpful. Indicating when they occur, and which source is overriding the others.

dnolen19:10:06

reporting errors is a second step

dnolen19:10:12

first step is semantics

dnolen19:10:24

it seems to me classpath stuff should always take precedence?

dnolen19:10:56

@mhuebert though I’m honestly still confused what problem you are running into

dnolen19:10:16

unless you have single-segment namespaces in ClojureScript

dnolen19:10:28

which you shouldn’t

dnolen19:10:59

what was the specific clash you encountered?

mhuebert19:10:54

react, react-dom

mhuebert19:10:03

I think what confused me was the precedence order

mhuebert19:10:24

I would expect a deps.clj file that I manually enter into a project to take precedence over ‘ambient’ deps in node_modules

mhuebert19:10:05

and precedence over cljsjs.react

mhuebert19:10:05

@dnolen i have been including cljsjs.react just for externs, and then using deps.clj to specify other files, then finding that the cljsjs stuff is what’s being included

mhuebert19:10:29

:global-exports seem often to be single-segment?

dnolen19:10:39

you’re confusing a lot of different things

dnolen19:10:50

:global-exports doesn’t have anything to do with what I’m talking about

dnolen19:10:27

you’re also talking about a problem specific to cljsjs.react and libs that make a similar sets of choices

dnolen19:10:46

was just trying to collect more information do understand what you perceived the problem to be

dnolen19:10:06

so that was helpful

mhuebert19:10:17

if I have (ns ... (:require [react])), and in deps.clj a foreign lib with a react global-export, and cljsjs.react included for externs purposes, in what order is react resolved?

dnolen19:10:41

probably the only thing we should do is warn and respect classpath (and other explicit) stuff

dnolen19:10:49

and libs can sort out their own problems

dnolen19:10:23

there are still edge cases though which it’s unclear how to handle

dnolen19:10:20

or at least it seems that way at the moment - can’t spend anymore cycles on this at the moment though

dnolen19:10:11

@mhuebert also I’m thinking using cljsjs.react only for externs is probably just not going to work if the cljsjs.react can be configured to consume react from different places

dnolen19:10:22

there’s just no way to say I just want this dep for externs

dnolen19:10:32

adding a knob for that also doesn’t seem like a particularly good idea to me at the moment

mhuebert19:10:15

so the decision of cljsjs.react to offer a react global-export is questionable - better to be cljsjs.react? then it would not collide with react in node_modules (https://github.com/cljsjs/packages/blob/master/react/build.boot#L60)

mhuebert19:10:52

current behaviour seems to do the equivalent of adding all node_modules package names, without any kind of prefix, into the ‘classpath’, and override whatever you might have manually specified in your own deps.clj file. this is a lot of names, which can change without any intent on the user, as transitive deps of anything you npm install get added there.

mhuebert20:10:36

I guess this is similar to how transitive deps of a clojure dependency are also added to the classpath. a bit worse because most node stuff isn’t namespaced

juhoteperi20:10:39

cljsjs/react and React node_modules must provide same name, so libraries (Reagent, Om) can be used with both

mhuebert20:10:47

@juhoteperi that makes sense. I guess it’s this automatic resolution that is confusing me.

mhuebert20:10:53

Is there a way to explicitly say, “`react` should come from node_modules” or “`react` is here in this file I am supplying myself” or “`react` is supplied as the global variable React“? I thought one could do that via deps.cljs/:foreign-libs, but it is overridden by node_modules. The resolution order is exactly the opposite of what I would expect. It’s currently 1. node_modules, 2. project.clj :dependencies, 3. the deps.cljs in my own project

dnolen21:10:10

@mhuebert right so that’s narrowing on the actual problem, I agree that order is not desirable

dnolen21:10:39

nor is it intended - and definitely not set in stone or anything

mhuebert21:10:39

@dnolen ok. i agree, the problem was not clear at first