Fork me on GitHub
#cljs-dev
<
2017-08-13
>
Oliver George00:08:25

I'm unable to use :npm-deps to use @blueprintjs.core. A warning is thrown and I can see that :js-dependency-index is missing a require.

Oliver George00:08:25

Would a JIRA ticket be welcome? I have a test which shows the problem. Not hard to describe here if that's preferred. (will make a thread on this to avoid being noisy)

Oliver George00:08:14

(deftest test-cljs-XXXX
  (test/delete-node-modules)
  (spit (io/file "package.json") "{}")
  (let [cenv (env/default-compiler-env)
        out (.getPath (io/file (test/tmp-dir) "test-npm-deps-order"))
        {:keys [inputs opts]} {:inputs (str (io/file "src" "test" "cljs_build"))
                               :opts   {:main             'npm-deps-test.test-cljs-XXXX
                                        :output-dir       out
                                        :optimizations    :none
                                        :install-deps     true
                                        :verbose          true
                                        :npm-deps         {:react     "15.6.1"
                                                           :react-dom "15.6.1"
                                                           :react-addons-css-transition-group "15.5.1"
                                                           "@blueprintjs/core" "1.24.0"}
                                        :closure-warnings {:check-types        :off
                                                           :non-standard-jsdoc :off}}}]
    (testing "@blueprintjs can require tslib"
      (test/delete-out-files out)
      (test/delete-node-modules)
      ; NOTE: Build warns about the issue
      (build/build (build/inputs (io/file inputs "npm_deps_test/bp_requires.cljs")) opts cenv)
      (let [tslib-module-entry (get (:js-module-index @cenv) "tslib")
            comp-module-entry (get (:js-module-index @cenv) "@blueprintjs/core/dist/common/abstractComponent")
            comp-dependency-entry (get (:js-dependency-index @cenv) (:name comp-module-entry))]
        ; NOTE: We can see in the deps index that the require was missed
        (is (contains? (set (:requires comp-dependency-entry)) (:name tslib-module-entry))))))
  (.delete (io/file "package.json"))
  (test/delete-node-modules))
(ns npm-deps-test.test-cljs-XXXX
  (:require ["@blueprintjs/core" :as blueprintjs]))

Oliver George00:08:42

This is the closure compile warning WARNING: JSC_JS_MODULE_LOAD_WARNING. Failed to load module "tslib" at /Users/olivergeorge/repos/clojurescript/node_modules/@blueprintjs/core/dist/common/abstractComponent.js line 9 : 4

Oliver George00:08:10

And this is the test output

@blueprintjs can require tslib
expected: (contains? (set (:requires comp-dependency-entry)) (:name tslib-module-entry))
  actual: (not (contains? #{"module$Users$olivergeorge$repos$clojurescript$node-modules$$blueprintjs$core$dist$common$utils" "module$Users$olivergeorge$repos$clojurescript$node-modules$react$react"} "module$Users$olivergeorge$repos$clojurescript$node-modules$tslib$tslib"))

anmonteiro00:08:23

Closure can’t understand tslib’s UMD wrapper

anmonteiro00:08:23

wait my assessment is wrong

anmonteiro00:08:39

the above is true, but there’s also a bug in CLJS and this should work with the latest Closure

Oliver George01:08:01

Thanks @anmonteiro. I was trying using clojurescript master. Seemed like tslib was coming though okay by itself.

anmonteiro01:08:33

the UMD problem is definitely happening

anmonteiro01:08:06

but we should be able to get around it, however we’re having another problem

Oliver George01:08:34

Thanks for looking.

anmonteiro04:08:22

@olivergeorge there might still be issues with the UMD thing (that need to be fixed in Closure), but your issue is fixed here: https://dev.clojure.org/jira/browse/CLJS-2318

Oliver George05:08:08

Confirming that improves my situation. No more warnings and the tslib related errors are gone.

Oliver George05:08:41

(still not a clean build but I wasn't expecting magic :-)

Oliver George05:08:11

next one seems to be tether's wrapper. i get "require not defined" after building. i assume another wrapper quirk https://unpkg.com/[email protected]

Oliver George05:08:32

i'll head back to #clojurescript for other q's

alex-dixon15:08:21

I’m thinking of filing an issue because alter-meta! does not work/behave like Clojure. Thought I would check here first after reading this https://dev.clojure.org/jira/browse/CLJS-1511

dnolen15:08:20

@alex-dixon there’s no reason for alter-meta! to work (precisely like Clojure)

dnolen15:08:24

there are no reified vars

dnolen15:08:06

in general the answer about reified var stuff from Clojure is “no”

dnolen15:08:51

we have a few affordances, but these are only for completely static cases (testing, cross module boundaries)

dnolen16:08:54

@alex-dixon metadata and vars are two completely separate things

dnolen16:08:01

so yes, that’s accurate

dnolen16:08:20

if you look at the documentation about vars you find that there are many caveats

dnolen16:08:52

fundamentally they don’t even exist

dnolen16:08:27

we just a do a tiny bit work to create the illusion for a couple of specific useful cases as I’ve said above

alex-dixon16:08:29

Is the docstring for alter-meta! considered accurate? “Atomically sets the metadata for a namespace/var/ref/agent/atom to be: (apply f its-current-meta args) f must be free of side-effects”

dnolen16:08:08

accurate modulo all other statements about vars not existing

alex-dixon16:08:33

Is altering the metadata of a var possible at runtime?

dnolen16:08:42

there are no vars

dnolen16:08:22

you can’t alter something that doesn’t exist

alex-dixon16:08:22

Sorry. I don’t understand “reified vars” and assumed it would be different from “var”. I thought reified vars don’t exist in CLJS

dnolen16:08:01

not different

dnolen16:08:05

there are no vars

dnolen16:08:19

means exactly the same thing as “there are no reified vars”

dnolen16:08:42

you can simulate some of the behavior by modifying the compiler state

alex-dixon16:08:45

Also confused I think by how I can do (meta foo) where foo is…something I’ve defined with def…and get CLJS metadata

dnolen16:08:17

(meta foo) doesn’t work

dnolen16:08:24

(meta #'foo) does work

dnolen16:08:41

but it’s an illusion, we just dump compiler information when we see #'foo

dnolen16:08:54

so completely static as I said earlier

dnolen16:08:13

so (alter-var! ...) couldn’t possibly do what you want

dnolen16:08:25

compile time only, it doesn’t matter where it appears

dnolen16:08:40

downstream observers would depend on compile order, not runtime

alex-dixon16:08:03

Ok. Thank you

dnolen16:08:21

but again, all of this goes back to something very simple

dnolen16:08:09

ClojureScript doesn’t have vars because it doesn’t make much sense with advanced compilation, code size, and performance as primary values

dnolen16:08:06

this has been true since day 1 when Rich Hickey designed ClojureScript

dnolen17:08:16

it’s turned out that a good many of the dynamic features of reified vars can be accomplished by other means

dnolen17:08:34

for example macros can get at the compiler environment

dnolen17:08:22

so as long as what you’re doing doesn’t fundamentally require runtime hijinks - there’s often another way to accomplish the same thing

alex-dixon17:08:31

In one version of what I’m trying to do, I can add the meta at compile time in a macro. Only thing is that there’s one symbol that I want to resolve to a value and I don’t know how to do that. So I changed my approach to try to have that code run at runtime instead

dnolen17:08:53

you can’t resolve to value (not generically)

dnolen17:08:05

and that hasn’t got anything do with vars or metadata

dnolen17:08:37

however if the value is a constant then what you want is trivial

dnolen17:08:48

@alex-dixon also it’s possible that what you want is bootstrapped ClojureScript

dnolen17:08:05

where you don’t have quite such a hard compile/runtime distinction

alex-dixon17:08:03

Maybe I can’t use metadata to store…metadata…because one of the values I need is realized at runtime, and I can only add metadata at compiletime, but…if I had the namespace and the symbol in the metadata, could I obtain the value from it at runtime?

dnolen17:08:46

hrm we’re going off into the weeds

dnolen17:08:04

instead of talking about solutions - just state at a high level what you are trying to accomplish

dnolen17:08:12

lets stop talking about vars, metadata, etc. it’s all a distraction

dnolen17:08:17

tell me you’re trying to do

alex-dixon17:08:42

I’m trying to make Precept manage multiple sessions (instances of Clara’s LocalSession). I wrote the initial code with references to global atoms, which worked fine if the library is only concerned about a single session . But now I want to manage multiple and pass them through everywhere. There’s data I need to associate with the session instances themselves. I don’t want to pass around a map because a lot of the code expects a LocalSession (and thought it would be the appropriate use of meta to do (meta #‘my-session) and get data back for its schema and a few other things. So, I’d prefer to pass around instances at least

dnolen17:08:15

right now that we’re rolling back - none of this sounds good to me 🙂

dnolen17:08:21

as I suspected

dnolen17:08:43

pass around map

dnolen17:08:50

stop creating fake problems

dnolen17:08:46

if your design work is leading you to want to a do a whole bunch of metaprogramming you can’t possibly be pursuing the right thing

dnolen17:08:57

in all of ClojureScript in the past six years we need metaprogramming in 2 places

dnolen17:08:10

testing (because we need to reflect on namespaces) and now module boundaries

alex-dixon17:08:41

I have all the data I need for a particular session inside the macro that defines it. The alternative seems a registry/lookup

dnolen17:08:57

registry/lookup seems fine

dnolen17:08:00

that’s how spec works

dnolen17:08:19

unless this somehow damages the API what’s the problem?

alex-dixon17:08:39

Yeah…I built on top of clara which uses macros a bit for DSL, compiling a rules engine into CLJS

dnolen17:08:53

DSL is fine, sugar stuff

dnolen17:08:02

but don’t build your system on macros

alex-dixon17:08:04

I don’t know. I thought it would be more pure functiony and less side-effecty

dnolen17:08:22

Clara may have some design issues here - I don’t know - I haven’t use it

dnolen17:08:55

if Clara doesn’t have desugared stuff

dnolen17:08:00

then that’s just broken

alex-dixon17:08:05

It’s incredibly well done imho if you ever have the time

dnolen17:08:09

that style of API is maddening

dnolen17:08:24

yes I’d like to take a look at it at some point

alex-dixon17:08:39

They designed it to be parallelizable. Part of the motivation for what I’m doing is to take advantage of that and have everything work in Clojurescript also

alex-dixon17:08:03

No clear reason why this was abandoned: https://github.com/rbrush/clara-storm

dnolen17:08:33

probably just a lot of work

alex-dixon17:08:21

But so is a lot of things, and I think there’s a lot of potential. Who knows though I guess

rads19:08:45

it looks like js/process is getting wiped out with {:target :nodejs :optimizations :simple} settings in the latest CLJS build (1.9.854) https://github.com/rads/cljs-bug

rads19:08:26

with 1.9.671 the js/process is not wiped out using the same settings

rads19:08:37

should I make a JIRA ticket for this?

dnolen22:08:59

ok only one :modules thing left, should be easy the dep order thing - will try to resolve that tomorrow

dnolen22:08:22

master has some changes to how we output under :none so that could use some testing under browser and Node.js

dnolen22:08:51

assuming :modules thing gets sorted tomorrow, can expect a release + updated docs / announce then

anmonteiro22:08:12

@dnolen is that the foreign libs thing?

anmonteiro22:08:15

the thing that’s missing

dnolen22:08:07

yeah but just dep order

dnolen22:08:50

should be quite easy to fix, I think module-graph should not assume the inputs order is correct

dnolen22:08:06

gotta head out

dnolen22:08:10

back on later

anmonteiro22:08:25

can’t actually repro that ^

anmonteiro22:08:45

I’ll attach a test case to CLJS-2309 that shows I can’t repro anymore

anmonteiro22:08:11

hrm actually I might be wrong here because I can’t repro with 1.9.854 either

anmonteiro22:08:03

anyway, test case patch attached to CLJS-2309

anmonteiro22:08:13

I think that should reproduce the issue (if there was one), but the test passes