Fork me on GitHub
#cljs-dev
<
2020-06-01
>
didibus05:06:10

Would it make sense for tools.deps to be extended to have a CommonJS Package Registry resolver?

didibus05:06:25

It would be a little tricky, since it would need to support package.json parsing to manage transitive deps as it does now for pom.xml with Maven registries

didibus05:06:28

But if it did, one could depend on NPM deps freely. And one could have a ClojureScript lib with NPM dependencies. And tools.deps would be able to do a full dependency closure on all of it and resolve conflicts, pull down the defined versions, etc.

dominicm09:06:41

Npm works differently to tdeps expectations because of hierarchical dependencies. Also, clojurescript doesn't search the classpath for node dependencies.

didibus21:06:02

Dependencies are hierarchical in tools.deps as well. Its a big tree

didibus21:06:01

And ya, I would assume the resolver would make symlinks into a local node_modules folder, or to be honest, it be nice if the compiler did look them up in the classpath, and when :bundle target is chosen it would go and create the node_modules symlinks needed for the js bundler to run over afterwards.

didibus21:06:36

I don't see much issue here. NPM seems pretty standard, it's got packages with versions, which can depend on other packages and optionally their versions if provided a package.lock

didibus21:06:08

At any rate, the compiler is just trying to re-implemented what looks to me like a worse version of tools.deps bit for cljs specifically. So I feel that effort would make sense to be consolidated into tools.deps

dominicm09:06:19

You can already depend on npm dependencies. I think it's deps.cljs on the classpath

bhauman16:06:38

OK so I the behavior I see when using the :bundle target is that cljsjs libraries are not connected to the required namespace symbol. When using reagent for instance, in the expression (:require [react]) the react symbol does not get bound to the cljsjs/react library when the react npm package has not been installed to node_modules.

bhauman16:06:53

what I see is an npm_deps.js with no entries, and in the compiled code I see reagent.impl.component.node$module$react = require('react');

bhauman16:06:04

Just verifying that this is the expected behavior

bhauman16:06:56

Folks probably need to understand the ins and outs of all this more as we move forward

dnolen16:06:23

@bhauman not expected, file an issue in JIRA this seems pretty minimal

dnolen16:06:55

basically if it's not in node_modules it should be resolved in the usual way

bhauman16:06:31

cool that’s what I thought the behavior should be

dnolen16:06:34

though you should check one thing

dnolen16:06:51

whether cljsjs.react has a deps.cljs with :npm-deps in it

bhauman16:06:11

reagent does of course

dnolen16:06:27

right so need to think about that then

bhauman16:06:56

cljsjs/react does not though

dnolen16:06:08

right sorry I mean reagent of course

bhauman16:06:15

oh makes sense

dnolen16:06:16

cljsjs/react wouldn't do that

dnolen16:06:54

yeah open to ideas here, not sure what makes sense

dnolen16:06:04

the first thing that comes to mind is :npm-deps supporting excludes

dnolen16:06:54

though my feeling is that this feels a bit like overkill

dnolen16:06:11

if you're going to use :bundle mixing it with CLJSJS seems silly anyway

dnolen16:06:30

especially if the lib you are using declared an npm dep

bhauman16:06:34

yes it does but dependencies are trees

dnolen16:06:55

but Reagent has done something funny

dnolen16:06:02

which is to declare two different deps

dnolen16:06:21

the same dep two very different ways

bhauman16:06:22

I thought that was the pattern to support both

dnolen16:06:32

not the way Reagent has done

dnolen16:06:38

Reagent has created a conflict

dnolen16:06:49

this is completely different from a user managing stuff

dnolen16:06:05

which is why I'm suggesting :excludes to remove an upstream conflict

bhauman16:06:56

So install-deps is inly intended to work for libraries that declare npm-deps by default

juhoteperi16:06:01

Huh, what's the problem?

dnolen16:06:36

the issue is that Reagent I believe both declared a dep on Maven dep and a NPM dep for the same library

dnolen16:06:45

in this case React

dnolen16:06:11

though ...

dnolen16:06:20

I guess we could only believe node_modules

dnolen16:06:23

and ignore :npm-deps

juhoteperi16:06:24

deps.cljs is only used if install-deps is enabled?

bhauman16:06:18

@juhoteperi we pinged you to find out something we answered already

bhauman16:06:56

@dnolen that makes sense to me

dnolen16:06:15

I audited the code a little bit

dnolen16:06:26

I'm not saying you aren't observing what you're observing

dnolen16:06:58

but I don't see how it wouldn't pick up cljsjs/react , it appears we do use node_modules as the source of truth

bhauman16:06:17

yeah it is strange for sure

dnolen16:06:38

please make something tiny and I can poke around - it's might be something else

bhauman16:06:04

I’ll put it in the jira

bhauman16:06:30

its just the simplest example without installing react via npm

bhauman16:06:47

i’m actually using the the webpack guide example

bhauman16:06:20

just skipping the npm add react react-dom step

dnolen16:06:20

but there are many strange things about what you're talking about - no errors

dnolen16:06:23

which seems impossible

dnolen16:06:36

it's either in node_modules or the classpath or you're going to get an error

bhauman16:06:52

I’m not excluding cljsjs

dnolen16:06:52

but that's what I'm talking about

dnolen16:06:58

so it must have found the classpath one

bhauman16:06:03

it does find the classpath one and includes it

bhauman16:06:17

but doesn’t bind it to the namespace symbol

bhauman16:06:15

BTW sorry for punctuating your days like this

dnolen16:06:37

no what I mean is there are two incompatible things

dnolen16:06:44

A) classpath library is found

dnolen16:06:12

B) node.js require emitted even though A) means it cannot be indexed under :node-module-index

thheller16:06:35

FWIW in shadow-cljs I completely dropped support for foreign-libs because I didn't want to deal with "mixing" dependencies. one library depending on cljsjs while the other going to npm directly. you might want to do the same for :bundle builds?

dnolen16:06:54

there's no need as far as I can tell as of yet

bhauman16:06:12

but there are foreign-libs that are just javascript includes for the local build

dnolen16:06:16

just a hassle for existing users who are transitioning too

bhauman16:06:55

yeah that’s the problem that someone came to me with this morning

dnolen16:06:11

in anycase what you're observing seems like an impossible situation - from looking over the code

dnolen16:06:25

you somehow have a node lib and a foreign dep

bhauman16:06:33

OK it’ll be in the JIRA

thheller16:06:41

I just wrote a stub library that maps the most common cljsjs package pack to a regular require and exposing the global

dominicm16:06:46

Could it be a transitive dependency?

dominicm16:06:03

Eg webpack depends on react or something

dnolen16:06:05

he never ran --install-deps so I doubt it

dnolen16:06:34

making :bundle work w/ everything that came before isn't that hard and is in the spirit of not breaking stuff that used to work

dnolen16:06:39

not going to change anything here

dominicm16:06:17

Must have installed webpack though, regardless of using cljs.main to do so?

bhauman16:06:45

let me detail the steps so there is little confusion

dnolen16:06:49

@bhauman you verified react isn't at the top level yes?

dnolen16:06:56

it does not matter who installed it by the way

bhauman17:06:10

I double checked it to make sure

dnolen17:06:02

thanks!!! will take a look

folcon17:06:46

Hey, I’m the one chatting to @bhauman. Wasn’t really expecting this to go here =)…

bhauman18:06:02

And here is another question: is this code broken or are we supporting it? https://github.com/JulianBirch/cljs-ajax/blob/master/src/ajax/xml_http_request.cljs#L29

bhauman18:06:21

basically its using cljs.core/*target* to detect if its in node.js

bhauman18:06:46

unfortunately under :bundle its “nodejs”

dnolen18:06:17

not really interested in making a new target

dnolen18:06:31

people should detect the platform by some other means

bhauman18:06:43

its a problem with the way its written

bhauman18:06:48

I was wrong

bhauman18:06:58

the require is emitted

bhauman19:06:18

either way so it doesn’t work in advanced

dnolen19:06:12

right that code just seems problematic

bhauman19:06:13

and we can override the *target* in closure defines

bhauman19:06:29

if we someone needs to

bhauman19:06:00

unfortunately a very popular library

dnolen19:06:58

I debated whether or not to actually emit a new target

dnolen19:06:10

but in the end that just means another thing for people to hook onto that they probably shouldn't

bhauman19:06:38

yeah I’m going to advise they use goog.global.require to not trigger the bundler, and use a different way to detect nodejs

thheller19:06:26

that won't work. require is not a global.

bhauman19:06:38

bad idea then

bhauman19:06:11

@thheller how do you solve this problem, you must have come across it

bhauman19:06:32

cljs-ajax specifically

thheller19:06:34

its not a problem since I don't extract js/require calls in the code for normal builds

thheller19:06:46

so for node builds it takes the correct path and just requires at runtime

thheller19:06:12

for browser builds it also does the correct thing and just uses XHR

bhauman19:06:26

and that’s the trick

bhauman19:06:54

do you special case other things besides XHR?

thheller19:06:19

no special case at all here. it is the logic in their code that does this.

thheller19:06:55

the problem only appears if you try to extract js/require calls when trying to feed them to the other bundler

thheller19:06:09

I only do this for react-native builds but not browser/etc

bhauman19:06:56

OK I’m trying to understand here the

bhauman19:06:14

CLJS is emitting require("xhr...")

bhauman19:06:24

and we pass that to the bundler

bhauman19:06:34

and the bundler does the wrong thing

bhauman19:06:57

I’m not understanding the word extract in this context

bhauman19:06:25

well we pass all the code to the bundler so that it resolves the requires

bhauman19:06:02

oh and I’m talking about advanced builds here

bhauman19:06:07

not normal builds

bhauman19:06:22

advanced or simple builds

thheller19:06:29

oh right .. well I don't pass anything to a bundler so the "rogue" require call does nothing

thheller19:06:49

its only for react-native builds that I actually extract js/require calls from the code since that will go through metro

thheller19:06:53

but all other builds won't

bhauman19:06:07

well that’s food for thought

bhauman19:06:08

and this case is particularly interesting because I don’t see how to make this code work at all now, if its getting passed to a bundler

bhauman19:06:18

a macro won’t work

bhauman19:06:30

because target is “nodejs”

dnolen19:06:44

to be honest I still think the library should just fix this

thheller19:06:52

yeah its not a great pattern but I've seen other libs doing this

bhauman19:06:52

the question is how?

dnolen19:06:56

because it's hard coded to a dynamic require

bhauman19:06:10

but how do they fix it?

thheller19:06:42

well its not a problem in shadow-cljs so doesn't require fixing. no clue about :bundle.

thheller19:06:43

but conditional requires are not fun to work with at all so I'd like there to be a better solution. just not sure how.

dnolen19:06:46

I don't have any interesting ideas that I'd be willing to implement or see implemented at the moment

dnolen19:06:29

the issue is way beyond this particular lib

dnolen19:06:44

any library that leaks js/require has this problem

bhauman19:06:08

they can’t use :npm-deps

bhauman19:06:22

they want a library thats cross-platform but it doesn’t look possible

dnolen19:06:39

doing this in JS wouldn't work either?

dnolen19:06:45

it's not specific to ClojureScript

thheller19:06:07

it does work in JS and is quite common. webpack eliminates some basic conditionals

dnolen19:06:29

"webpack eliminates some basic conditionals" != "it does work"

dnolen19:06:36

you mean some cases work no?

dnolen19:06:00

but that honestly looks like the answer

thheller19:06:00

yes, it works for some cases

dnolen19:06:03

not our problem

dnolen19:06:05

do what JS does

thheller19:06:28

problem is that it likely won't understand the cljs.core/*target* conditional. I have only seen this work for actual const locals and process.env.NODE_ENV

dnolen19:06:37

but that's what I mean

bhauman19:06:00

oh OK then that’s doable

dnolen19:06:04

you would emit that process.env.NODE_ENV

dnolen19:06:13

the important thing here is this

dnolen19:06:29

:bundle has to mean stuff that Webpack could consume sensibly if used

dnolen19:06:40

so for a library to adopt :bundle it has to adopt these bundler conventions

dnolen19:06:41

about platforms

dnolen19:06:48

inventing stuff is not that appealing IMO

thheller19:06:57

NODE_ENV is only about development and production though

thheller19:06:44

you can always ignore the xmlhttprequest require via webpack config when building for the browser

bhauman19:06:10

oh well that’s interesting as well

thheller19:06:36

resolve: {"xmlhttprequest":false} or whatever the config for that was

bhauman19:06:37

resolve alias

thheller19:06:39

yeah I have something similar in shadow-cljs https://shadow-cljs.github.io/docs/UsersGuide.html#js-resolve .. for some npm packages there is just no other way without manual intervention

bhauman19:06:57

good stuff

dominicm19:06:15

That means that libraries couldn't change their underlying library without breakage. But that's js too

bhauman19:06:00

so yeah day8.re-frame/http-fx doesn’t work for advanced because

dnolen19:06:54

but if the underlying lib fixes the issue then it goes away right?

dnolen19:06:12

to be honest I don't see any simple way to fix this

dnolen19:06:31

you could legitimately sprinkle your code with js/require knowing you will use a bundler

dnolen19:06:59

btw, above I wasn't shooting down ideas to fix it - just stating I don't have any good ones

dnolen20:06:23

which is why I was suggesting adopting bundler conventions

dnolen20:06:55

because I'm afraid we would just interfere with established conventions

jsa-aerial20:06:19

Can anyone here shed some light on an aspect of the compiler analysis state cache? In particular, in the following JS console inspect snippet, • why isn't the macros field at the "top" level (same level as extrerns? • how can you have multiple null keys in the map and what does that entail?? • what does edit mean?

6: {ns: null, name: "cljs.core.async", str: "cljs.core.async", _hash: -159169011, _meta: null, …}
7:
meta: null
cnt: 16
root:
edit: {}
bitmap: 221921953
arr: Array(24)
0: null
1: {edit: {…}, bitmap: 34603008, arr: Array(8), cljs$lang$protocol_mask$partition1$: 131072, cljs$lang$protocol_mask$partition0$: 0}
2: {ns: null, name: "externs", fqn: "externs", _hash: 221720677, cljs$lang$protocol_mask$partition0$: 2153775105, …}
3: {meta: null, cnt: 3, arr: Array(6), __hash: null, cljs$lang$protocol_mask$partition0$: 16647951, …}
4: null
5:
edit: {}
bitmap: 3211264
arr: Array(8)
0: {ns: null, name: "use-macros", fqn: "use-macros", _hash: -905638393, cljs$lang$protocol_mask$partition0$: 2153775105, …}
1: {meta: null, cnt: 2, arr: Array(4), __hash: null, cljs$lang$protocol_mask$partition0$: 16647951, …}
2: {ns: null, name: "excludes", fqn: "excludes", _hash: -1791725945, cljs$lang$protocol_mask$partition0$: 2153775105, …}
3: {meta: null, hash_map: {…}, __hash: null, cljs$lang$protocol_mask$partition0$: 15077647, cljs$lang$protocol_mask$partition1$: 139268}
4: {ns: null, name: "macros", fqn: "macros", _hash: 811339431, cljs$lang$protocol_mask$partition0$: 2153775105, …}
5:
meta: null
cnt: 3
arr: Array(6)
0: {ns: null, name: "go", str: "go", _hash: 1493584872, _meta: null, …}
1: {meta: null, cnt: 8, arr: Array(16), __hash: null, cljs$lang$protocol_mask$partition0$: 16647951, …}
2: {ns: null, name: "alt!", str: "alt!", _hash: 1759993452, _meta: null, …}
3: {meta: null, cnt: 8, arr: Array(16), __hash: null, cljs$lang$protocol_mask$partition0$: 16647951, …}
4: {ns: null, name: "go-loop", str: "go-loop", _hash: 692273294, _meta: null, …}
5: {meta: null, cnt: 8, arr: Array(16), __hash: null, cljs$lang$protocol_mask$partition0$: 16647951, …}
The context for this is getting the macros to be visible to the compiler

jsa-aerial20:06:36

An odd (to me) thing is if you look at this in cljs-repl there are no null keys and :macros is at the top level of the map

thheller20:06:35

you are looking at the internal structure of the map implementation. thats not very interesting. use cljs-devtools or just prn the result to get a useful representation

jsa-aerial20:06:04

As I mentioned, the 'useful' representation has the :macros field at the top level of the map. Here's a snippet:

((-> state deref :cljs.analyzer/namespaces) 'cljs.core.async)
{:rename-macros {},
 :renames {},
 :externs {Error {}, Array {}, Object {}},
 :use-macros {go cljs.core.async, go-loop cljs.core.async},
 :excludes
 #{reduce take map transduce into partition merge partition-by},
 :macros
 {go
  {:arglists ([& body]),
   :doc
   "Asynchronously executes the body, returning immediately to the\n  calling thread. Additionally, any visible calls to <!, >! and alt!/alts!\n  channel operations within the body will block (if necessary) by\n  'parking' the calling thread rather than tying up an OS thread (or\n  the only JS thread when in ClojureScript). Upon completion of the\n  operation, the body will be resumed.\n\n  Returns a channel which will receive the result of the body when\n  completed",
   :line 4,
   :column 1,
   :file "cljs/core/async.clj",
   :name cljs.core.async/go,
   :ns cljs.core.async,
   :macro true},
  alt!
  {:arglists ([& clauses]),
   :doc
   "Makes a single choice between one of several channel operations,\n  as if by alts!, returning the value of the result expr corresponding\n  to the operation completed. Must be called inside a (go ...) block.\n\n  Each clause takes the form of:\n\n  channel-op[s] result-expr\n\n  where channel-ops is one of:\n\n  take-port - a single port to take\n  [take-port | [put-port put-val] ...] - a vector of ports as per alts!\n  :default | :priority - an option for alts!\n\n  and result-expr is either a list beginning with a vector, whereupon that\n  vector will be treated as a binding for the [val port] return of the\n  operation, else any other expression.\n\n  (alt!\n    [c t] ([val ch] (foo ch val))\n    x ([v] v)\n    [[out val]] :wrote\n    :default 42)\n\n  Each option may appear at most once. The choice and parking\n  characteristics are those of alts!.",
   :line 63,
   :column 1,
   :file "cljs/core/async.clj",
   :name cljs.core.async/alt!,
   :ns cljs.core.async,
   :macro true},
  go-loop
  {:arglists ([bindings & body]),
   :doc "Like (go (loop ...))",
   :line 95,
   :column 1,
   :file "cljs/core/async.clj",
   :name cljs.core.async/go-loop,
   :ns cljs.core.async,
   :macro true}},
 :name cljs.core.async,
But the compiler claims, for example, that the go macro does not exist in the cljs.core.async namespace

thheller20:06:00

this is self hosted and core.async isn't self-host compatible

jsa-aerial20:06:50

I am using Andare which is supposed to be self-host compatible. I did discuss a bit with mfikes, but it seemed the issue was more related to the compiler

jsa-aerial20:06:16

I find it strange that the internal structure differs in the simple respect that the array map impl has (multiple?) null keys

thheller20:06:34

don't look at the JS representations of the CLJS data structures. they are just going to confuse you. :P

thheller20:06:56

they are null in clojure too .. you are just not looking at the internal java representation ...

thheller20:06:33

eg. {ns: null, name: "cljs.core.async", str: "cljs.core.async", _hash: -159169011, _meta: null, …} ... this is the cljs.core.async symbol but its really hard to tell that from the output

jsa-aerial20:06:22

Yeah, understand that - but it does not have null keys

thheller20:06:33

watch some talks or read some blog posts about how the CLJ(S) data structures work. it'll make more sense then.

👍 4
jsa-aerial20:06:53

Putting that aside then (representation aspects), it would then seem that the compiler state does have the macros as in the cljs.core.async name space. But any attempt to reference them gives an analyzer error that the do not exist

thheller20:06:36

for self hosted the macros will live in cljs.core.async$macros but I'm honestly not entirely sure how self-hosted macros work

thheller20:06:58

but I do know that they live in a secondary namespace

jsa-aerial20:06:24

Yeah, that stuff is there as well - and the macros are listed in there too

thheller20:06:46

I don't know what you actual question is though. the JS "mess" you posted doesn't really tell you anything about the data you actually have. just pprint it or something to get a useful view of the data

thheller20:06:01

trying to deciper the JS objs is just not useful

jsa-aerial20:06:25

??? I posted the actual 'useful view' of the compiler state above

thheller20:06:46

ah thought you got that from somewhere else

jsa-aerial20:06:00

No, that is the real live data

lilactown20:06:29

@jsa-aerial it sounds like you’re trying to debug some issue with andare. Like @thheller is saying, the three questions you posted above about the JS is a red herring, it probably has nothing to do with your issue. can you explain the issue you’re having with andare?

jsa-aerial21:06:33

As mentioned at 4:51 msg, despite the macros being in the compiler state for cljs.core.async, the analyzer claims they do not exist

jsa-aerial21:06:54

Maybe this is just a fool's errand and unless you know some deep (and apparently opaque) magic, it just isn't possible to get this to work

thheller21:06:40

where or how does it claim it? maybe your use is just incorrect?

thheller21:06:45

but self-host macros are definitely magic ... 🙂

jsa-aerial21:06:08

In a namespace if you issue (require '[cljs.core.async :refer-macros [go]]) for example, it just errors with illegal refer that macro cljs.core.aync/go does not exist.

thheller21:06:56

what about (require '[cljs.core.async :refer [go]]) or (require-macros '[cljs.core.async :refer [go]])?

thheller21:06:34

also I'm assuming you are talking about a REPL right? otherwise requires should be in the ns

jsa-aerial21:06:51

You can (require-macros '[cljs.core.async.macros :refer [go]]) w/o analyzer error, but any attempt to use it (for example with <!) just errors with <! not in a go block.

thheller21:06:55

where does cljs.core.async.macros suddenly come from?

jsa-aerial21:06:42

That's the separate namespace you mentioned - cljs.core.async require-macros that

thheller21:06:28

no I didn't. I said cljs.core.async$macros. which is different. thats the "secondary" namespace self-hosted will be using for macros

thheller21:06:50

cljs.core.async.macros is another actual ns, which will also have the secondary cljs.core.async.macros$macros

thheller21:06:10

again ... macros in self-hosted are weird 😛

jsa-aerial21:06:19

> also I'm assuming you are talking about a REPL right? otherwise requires should be in the ns either repl or ns - either way behaves the same

jsa-aerial21:06:44

Well, there is no cljs.core.async$macros in any of the code

thheller21:06:52

then there is your problem

thheller21:06:04

well it shouldn't be in your code .. but it should be in the analyzer state

jsa-aerial21:06:52

OK, that seems to be how Andare is written. It uses this cljs.core.async.macros namespace

jsa-aerial21:06:04

It's definitely not in the analyzer state. Does that get generated or is it how the code is expected to be written?

thheller21:06:43

that you'll have to ask someone that actually knows how self-hosted works 😛

lilactown21:06:58

does (require '[cljs.core.async :refer [go]]) not work?

jsa-aerial21:06:09

It does not work

jsa-aerial21:06:30

It just says it does not exist

thheller21:06:32

if the analyzer state has no cljs.core.async$macros then it does in fact not exist

jsa-aerial21:06:06

Well, I guess this just goes back to the 'deep / opaque' magic bit. Andare does not have any such namespace (that I can find) but the claim is that it does work

thheller21:06:51

it is the namespace the is created when the .clj files are compiled as macros

jsa-aerial21:06:10

Oh, so it is generated

thheller21:06:24

it doesn't exist on disk yes ... it is the result of compiling cljs/core/async.clj as a macro namespace

jsa-aerial21:06:49

Well, there you are - maybe I just need to grab and cache that at compile time

thheller21:06:36

probably, no clue how self-host handles that.

jsa-aerial21:06:22

Let me try that and see what happens

didibus22:06:40

Wouldn't that kind of conflict be something that would be solved if we reconcile it all in tools.deps? Tools.deps could find conflicts between a cljsjs and a node_modules or a git deps, etc. And just resolve which one to pull down

jsa-aerial22:06:58

> maybe I just need to grab and cache that at compile time Nope that doesn't work either. In fact the compiler state at build time claims there is no cljs.core.async$macros namespace. I think I am going to give up and approach originating problem in a (less general, less simple, and less clean) fashion - but which should work, as I will be using core.async at the app (non self hosted) level (which definitely does work). 😒 this was one of the more total wastes of time I've had the misery to have engaged in.