Fork me on GitHub
#cljs-dev
<
2017-01-28
>
dnolen00:01:10

I don’t think that’s true anymore but you should double check

dnolen00:01:23

I’m also curious whether the flattening thing helps here

dnolen00:01:29

anyways try it out and report back

pat02:01:13

I'm seeing 'dirname is not defined' with fw on nodejs. The only thing I can think of is that dirname is context sensitive (I think?) and fw's client breaks the context. anyways, reverting the bootstrap node changes in 59a7f26 fixes

dnolen02:01:00

@pat sounds like something to report to figwheel and then see if it can’t be sorted there first

anmonteiro02:01:16

@dnolen might be the same issue as require

anmonteiro02:01:36

I think __dirname and __filename may need to be bound to global

dnolen02:01:38

oh right writing it to global

dnolen02:01:42

@anmonteiro I don’t know about filename since that needs to be done by require

dnolen02:01:09

hrm, but I guess we should bind it at root, and expect require to do the right thing later

dnolen02:01:28

cool, happy to take a patch for that

anmonteiro02:01:41

@dnolen I'll have something for you tonight

dottedmag07:01:19

@dnolen A typo in externs guide: Cannot infer target type in expression (.bar x) ... -- the code above calls (.baz x).

anmonteiro07:01:18

I haven't, not yet

anmonteiro07:01:31

having trouble?

martinklepsch07:01:39

@anmonteiro an idea by any chance how that deps.cljs has been generated?

martinklepsch07:01:18

I’m currently looking into how cljsjs could utilize the new stuff. Your changes to Om seem like it would be relatively straightforward, the es6-demo however seems very different (i.e. all files are provided as foreign lib individually)

anmonteiro07:01:45

going to bed now, curious to see what you come up with. Hope the above helps

martinklepsch07:01:09

Well, don’t expect too much haha 😄

martinklepsch07:01:11

Good night 🙂

martinklepsch07:01:14

Thanks for the pointer

martinklepsch08:01:49

doh, just node test.js 😉

Roman Liutikov09:01:24

got React production build working with env variable set via

(ns process.env)

(goog-define NODE_ENV “development")
:closure-defines {'process.env/NODE_ENV "production"}

Roman Liutikov09:01:59

React + ReactDOM no env var 71KB gzipped env set to “development” 69KB gzipped env set to “production” 52KB gzipped

Roman Liutikov09:01:45

it’s funny that development build with env var set has less code that the one without 😄

Roman Liutikov09:01:39

however, Webpack production build is 42KB gzipped

thheller11:01:02

@dnolen trying the latest cljs with shadow-build, I get literally thousands of infer warnings although I did not enable it anywhere WARNING: Cannot infer target type in expression (. cljs.core/PersistentVector -EMPTY-NODE) ...

thheller11:01:08

is this intented to be always running?

thheller11:01:38

probably missing some setup in shadow-build but I'm confused why this code is reached at all

thheller11:01:24

@dnolen not shadow-build related, if you actually enable :infer-warnings globally you get many many warnings everywhere

thheller11:01:50

would produce warnings for cljs.core as well but it is using the cached version so it is hidden

thheller11:01:00

so the warning is always generated, it is just silenced when *warn-on-infer* is not set

thheller11:01:06

that doesn't seem right to me

bhauman14:01:54

looks like only a tiny figwheel tweak to get file reloading on dir based foreign libs

bhauman14:01:32

Right now I'm adding foreign libs to the watched paths by default, hasn't been a problem so far. But I'm think I'm going to need to make this explicit instead of implicit.

dnolen14:01:25

hilarious that’s so easy 🙂

dnolen14:01:44

@thheller :infer-warnings is not intended to ever be set

martinklepsch14:01:54

@roman01la can you share your project maybe?

dnolen14:01:03

it’s useless when applied globally, the guide says this

dnolen14:01:59

@martinklepsch I would just refer to the guide now, it covers everything from es6-demo but with like - documentation

dnolen14:01:21

happy to answer any questions that you may have about how it works

dnolen14:01:45

basically we shell out to node to run module-deps on a your specified CommonJS entry point

dnolen14:01:06

that returns a JSON value of the dependencies, an array

martinklepsch14:01:15

the interesting part about the es6-demo was how it integrated React or “npm library code” more generally

thheller14:01:16

@dnolen that is not what I meant. I meant that there is no way to actually toggle :infer-externs as off, the work is always done.

dnolen14:01:16

we just map over it grabbing ”file”

thheller14:01:45

which I disagree with as a concept

dnolen14:01:37

@thheller right we do the work anyway - not a big deal, but easy to fix in a patch

dnolen14:01:03

@martinklepsch but the JS guide covers that as the last thing - I don’t know what you mean

martinklepsch15:01:26

I did update the deps.json by running node test.js > deps.json but got stuck after a while with this one:

ERROR: JSC_COMMONJS_MODULE_LOAD_ERROR. Failed to load module "react" at /Users/martin/code/es6-demo/src/libs/NodeStuff.js line 2 : 12
Exception in thread "main" java.lang.IllegalStateException: Cannot build without root node being specified
I read the last section on the docs page but it doesn’t give much more info than “it might not be possible"

martinklepsch15:01:04

My motivation of using es6-demo was to have something working and tinker with that as foundation

dnolen15:01:40

@martinklepsch I keep saying that the guide shows you how to do all this 🙂

dnolen15:01:56

I don’t know why you are struggling with the es6-demo 🙂

dnolen15:01:20

@martinklepsch which part “might not be possible"

dnolen15:01:25

are you referring to in the guide?

bhauman15:01:36

@dnolen old foreign libs behavior like:

dnolen15:01:57

the reason you’re failing at that spot is that module-deps doesn’t add package.json entries

dnolen15:01:14

I got it working by manually add the one for React

dnolen15:01:19

the guide one just works

dnolen15:01:41

because in cljs.build.api we add the package.json entries after calling module-deps

bhauman15:01:04

That ^ should work right?

bhauman15:01:50

shoot skip the :as React

dnolen15:01:32

maybe but it’s hard to say I think using es6-demo here is very counterproductive for everyone 🙂

martinklepsch15:01:14

I think it would be cool to have it working so it can serve as a playground for people interested in messing around with the module stuff

bhauman15:01:51

I'm pretty sure that what I'm doing should work

dnolen15:01:56

@martinklepsch right, but I don’t know why you can’t do that with the final example in the guide is all I’m saying

martinklepsch15:01:00

> the reason you’re failing at that spot is that module-deps doesn’t add package.json entries you mean as in it fails because react isn’t in package.json? (it is so I guess I’m misunderstanding)

dnolen15:01:18

es6-demo was a hack and I didn’t document all the details that I fixed in ClojureScript itself

dnolen15:01:34

everything I learned was put into ClojureScript directly and automated

dnolen15:01:18

@martinklepsch so let’s rewind and let me explain how it works

dnolen15:01:34

Google Closure has Node.js resolution for CommonJS inputs

dnolen15:01:39

by resolution I don’t mean finding files

martinklepsch15:01:39

In es6-demo could I require react from core.cljs? I think this is where my misunderstanding might stem from

dnolen15:01:26

@martinklepsch only if you supply a :provides

martinklepsch15:01:41

right, ok. I think I’m slowly catching up 🙂

dnolen15:01:05

so Google Closure expects to see all of your inputs

dnolen15:01:17

the problem is we don’t know what they are!

dnolen15:01:32

we need some other tool to figure out what the CommonJS deps are

dnolen15:01:37

module-deps does this

dnolen15:01:09

so ClojureScript shells out to get the CommonJS deps so we can give the Closure Compiler all the inputs

dnolen15:01:25

but it needs to do more, it needs also to include package.json entries

bhauman15:01:26

@dnolen what I'm seeing it that :provides [React] is not putting an entry in cljs-deps.js

dnolen15:01:42

because those tell us what main: is

martinklepsch15:01:57

ok, 1s here. What needs to include package.json entries? and what is main: in that context? is that a node/npm thing?

dnolen15:01:37

@martinklepsch Node.js dep resolution

martinklepsch15:01:07

ok, so that’s what goes into require() essentially

dnolen15:01:19

in order find the entry point you may need to look at package.json

dnolen15:01:29

I’m not talking about top level package.json

dnolen15:01:37

I mean node_modules/react/package.json

dnolen15:01:01

which says main is node_modules/react/react.js (not index.js the default)

martinklepsch15:01:21

oh, ok, that clears up the confusion about that part

dnolen15:01:06

@bhauman that’s not how :provides ever worked in Maria’s code

bhauman15:01:13

ok I'm getting there

dnolen15:01:23

so another explanation which might clarify things

bhauman15:01:24

yep just figured that out

bhauman15:01:28

sorry for noise

dnolen15:01:56

no problem 🙂

dnolen15:01:14

I’m enjoying the enthusiasm that people have for figuring how this stuff works 🙂

dnolen15:01:13

this is exactly what's missing in es6-demo

dnolen15:01:18

that’s why you can’t load React

martinklepsch15:01:58

@dnolen yup, the pieces start to fit a bit more. I think you gave me enough to chew on for now. Thanks for taking the time to explain! I’m sure I’ll be back though 🙂

bhauman15:01:31

@dnolen OK I see the problem I'm having

bhauman15:01:52

I think I have a better question

bhauman15:01:41

actually I think I may have answered it for myself

bhauman15:01:51

the react in es6 demo

bhauman15:01:05

actually let me explore more

bhauman15:01:57

it seems the example above

bhauman15:01:03

the common js example

bhauman15:01:13

is compiling a no op

bhauman15:01:34

goog.provide("module$resources$js$react");var module$resources$js$react={};
(function(f){var module$resources$js$react=f()})(function(){var define,module,exports;re

bhauman15:01:59

note the var assignment

dnolen15:01:05

“is compiling a no op”?

bhauman15:01:17

see the var assignment in the function

bhauman15:01:35

the react module is never required

bhauman15:01:50

or never put into the global env

dnolen15:01:57

I don’t know what you mean

bhauman15:01:08

oh darn this is hard to exxpresss

dnolen15:01:21

the code there is just Google Closure rewriting that CommonJS module into a Google Closure namespace

dnolen15:01:28

we’re not involved in that

bhauman15:01:09

so my snippet before no longer works

bhauman15:01:14

so what I'm saying is that :provides used to reify module$resources$js$react into the global env

bhauman15:01:34

and no longer does

dnolen15:01:44

hrm I don’t see how

dnolen15:01:50

var module$resources$js$react={};

dnolen15:01:55

is exactly that

bhauman15:01:29

OK I'll look further

dnolen15:01:31

if not handled directly by goog.provide

dnolen15:01:16

@bhauman but let me see if I can see what you’re seeing

dnolen15:01:40

are you using the latest React that you downloaded or something?

bhauman15:01:25

I'm using the older version of es6-demo

bhauman15:01:29

the react there

bhauman15:01:51

and I'm using the :provides the old way

bhauman15:01:24

you may want to let me confirm that this behavior changed first

bhauman15:01:04

@dnolen yes the behavior changed

bhauman15:01:23

in cljs 1.9.303

bhauman15:01:47

the top level var get's assigned when :provides is used the old way

bhauman15:01:22

actually 1.9.293 behaves this way as well

dnolen15:01:06

@bhauman right but we can’t do anything about that

dnolen15:01:16

that’s just Google Closure stuff

dnolen15:01:29

and you don’t need the var

dnolen15:01:36

goog.provide creates it

dnolen15:01:59

Google Closure CommonJS compilation changed here - nothing in ClojureScript

bhauman15:01:57

well I can't require it or reference it

bhauman15:01:13

it's hermetically sealed

dnolen15:01:08

right this is something we discussed a long time ago but maybe it wasn’t clear then

dnolen15:01:15

we don’t know what the name is going to be

dnolen15:01:47

Maria does preserve that original provides information in the compiler as an alias

dnolen16:01:02

so we can map from Closure’s generated name to the one specified by the user

bhauman16:01:01

OK I'm getting there

bhauman16:01:15

it is a novel behavior change in the relationship between require/:provides in foreign libs for common js

dnolen16:01:59

@bhauman it’s been like this since Maria implemented it 1 1/2 years ago 🙂

dnolen16:01:15

nothing has changed

bhauman16:01:22

then we aren't communicating verywell

bhauman16:01:36

even if the change is on closure proper

bhauman16:01:40

its a novel change

dnolen16:01:00

I think you are still confused here

bhauman16:01:14

I used to be able to require and refer to the provided name

dnolen16:01:14

goog.provide always create the top level object

dnolen16:01:38

someone at Google wrote a commonJS thing that generated a top level var

bhauman16:01:39

and it would have React in it

dnolen16:01:51

someone realized it was redundant, and removed that

dnolen16:01:54

none of this matters at all

dnolen16:01:10

Maria’s patch realized that module names are generated

dnolen16:01:14

it was always this way

dnolen16:01:38

so she preserves the foreign lib :provides and maps to the generated one

dnolen16:01:48

nothing has changed here still

dnolen16:01:50

this is all new stuff

dnolen16:01:00

what you’re talking about is normal foreign libs

dnolen16:01:23

so there are no changes here

dnolen16:01:27

same stuff as it was the whole time

dnolen16:01:42

@bhauman so the problem you have now is how to require a thing at runtime

dnolen16:01:55

for this new kind of compiled thing

dnolen16:01:38

fortunately I think this is completely solveable

dnolen16:01:51

we generate cljs_deps.js

dnolen16:01:07

a file can have multiple provides

dnolen16:01:16

we just need to dump the original alias

dnolen16:01:22

@bhauman do you agree that’s all you really need?

bhauman16:01:48

yes, I think, a way to require the :commonjs module

bhauman16:01:03

but I'm not super frustrated with this

bhauman16:01:20

I don't use common js modules

dnolen16:01:55

right there are harder problems to sort out here that we haven’t even gotten to yet as well

dnolen16:01:06

(ns foo.core (:require [object-assign]))

bhauman16:01:08

I think I just got hung up on some transient behavior in 1.9.303

dnolen16:01:08

doesn’t work

dnolen16:01:55

@bhauman right, I think for Figwheel you just need the original aliases in the goog.addDependency(… calls

bhauman16:01:45

I'm not even there yet

bhauman16:01:13

figwheel should work just fine with all this

bhauman16:01:40

it uses the generated aliases

dnolen16:01:46

hrm ok then I don’t think we need to do anything at all then right?

dnolen16:01:53

require etc. already works

dnolen16:01:29

just trying to figure if you actually need something 🙂

bhauman16:01:49

I'm at a cafe, if I go someplace where we can chat, do you have a minute?

bhauman16:01:22

oh wait ... I think I got this

dnolen16:01:40

@bhauman happy to chat if it makes it easier

dnolen16:01:29

yeah just ping me on skype if you want

bhauman16:01:06

right so I'm going to get off of this common js stuff for the moment and just get things in figwheel working for dir based foreign libs

dnolen16:01:53

React + ReactDOM Server with advanced compilation 32 K 🙂

dnolen16:01:05

21K smaller than what FB provides

dnolen16:01:33

@roman01la the reason you weren’t seeing this result is because of :target :nodejs

dnolen16:01:48

and don’t use println

dnolen16:01:58

then you’ll see Google DCE really kick in

Roman Liutikov16:01:15

@dnolen: what does it mean "what FB provides"

dnolen16:01:32

just production scripts

dnolen16:01:42

react + react dom server

Roman Liutikov16:01:47

Ah, ok. The stats I reported was for React + React DOM browser, want using Node as a target or println

Roman Liutikov16:01:53

There was also a diff between Webpack and cljs build in 10 kb, I think that's because of runtime overhead, but @martinklepsch pointed out that cljs runtime should be elided, so I'm not entirely sure if Closure Compiler can optimize React as JS optimizer, but that doesn't make sense to me

dnolen16:01:44

the numbers I reported show that Closure can optimize React

dnolen16:01:17

it also appears FB is coming to same conclusion

Roman Liutikov17:01:18

I've used Webpack 2, not sure how Facebook do it

Roman Liutikov17:01:38

So there might be a runtime or Webpack 2 can elide more stuff

Roman Liutikov17:01:53

Not sure how to compare both builds properly to find out

anmonteiro17:01:50

@roman01la: I thought they used browserify

anmonteiro17:01:59

Should be in the React repo somewhere

dnolen17:01:33

to me the coolest thing is that stuff like this is so easy to try/test now 🙂

dnolen17:01:42

such a massive pain before

anmonteiro17:01:06

Not sure how worthy the comparison between webpack/browserify is though....

Roman Liutikov17:01:29

@anmonteiro: I mean Webpack vs Closure

anmonteiro17:01:05

I was answering to "not sure how FB do it"

Roman Liutikov17:01:43

Yeah, things are much better now. @dnolen are there any ideas on how to make it possible to consume NPM deps directly, without JS entry point?

dnolen17:01:01

@roman01la task in JIRA about that

Roman Liutikov17:01:04

Probably a lookup for every package json

dnolen17:01:08

will require hammock time

Roman Liutikov17:01:25

Ok, will check it, thanks.

dnolen17:01:58

based on what Chad Killingsworth said this may be easier than it sounds, dependency mode STRICT implies all of the machinery is already present

Roman Liutikov17:01:57

I was wrong! And I’m glad I was 😄 React + ReactDOM 32KB vs Webpack 42KB

Roman Liutikov17:01:30

probably cljs build caches was the cause

anmonteiro17:01:30

@dnolen: that JIRA ticket above... by "directly requiring" you mean without needing to add a :provides entry?

bhauman17:01:13

ok I've got hot reloading working when foreign lib :file is a dir

bhauman17:01:03

wow its really working well

bhauman17:01:14

So what's the best way most common way for me to require React in my :es6 hello world

Roman Liutikov17:01:13

@bhauman I’ve just published a test repo, you can see how it’s done there https://github.com/roman01la/cljs-npm-test

bhauman17:01:33

You may have noticed that our ES6 file does not declare its dependency on React, ReactDOM, or ReactDOMServer via import. Handling this correctly depends on a pending patch to Google Closure to support Node.js module resolution for ES6 source files. When this change lands this guide will updated.

dnolen18:01:51

@roman01la that’s what I was saying 🙂 but glad you could repro

dnolen18:01:25

@anmonteiro I mean treating CommonJS modules as Google Closure namespaces

dnolen18:01:45

right now you have to go through a CommonJS entry point

dnolen18:01:19

@bhauman yeah waiting on Closure Compiler here

bhauman18:01:40

@dnolen and I can't require react via the commonjs module like you did in the es6-demo either

dnolen18:01:24

@bhauman I don’t know what you mean

bhauman18:01:42

let's skype!

dnolen18:01:58

haha can’t now

bhauman18:01:04

oh bummer 😞

dnolen18:01:05

making breakfast

bhauman18:01:14

cool Imma get lunch

anmonteiro19:01:33

bummer we'll have to wait ~1 month for a release

anmonteiro19:01:39

that commit missed the Jan release by 3 days

dnolen19:01:35

@anmonteiro it’s February in a couple of days 🙂

dnolen19:01:57

and it sounds like we’ll need to cut a new release soon anyhow

anmonteiro19:01:04

right, but we need to wait until the end of February 🙂

dnolen20:01:19

@bhauman did you sort out your issue?

bhauman20:01:46

maybe we could chat for a sec?

bhauman20:01:30

do you have screenhero?

dnolen20:01:40

I don’t just Skype

anmonteiro21:01:39

@dnolen weird edge case that works in Clojure but not CLJS:

cljs.user=> (def foo '(:a :b))
#'cljs.user/foo
cljs.user=> (case foo '(:a :b) :works)
Error: No matching clause: (:a :b)

anmonteiro21:01:34

found it because inadvertently quoted the list passed to case when I really wanted a non-quoted list

dnolen21:01:40

File a bug thanks

Alex Miller (Clojure team)23:01:24

I don't think that should work in Clojure

Alex Miller (Clojure team)23:01:01

Lists in case represent a list of cases, not sure if you're aware of that

Alex Miller (Clojure team)23:01:41

You can't match a literal list in case afaik

anmonteiro23:01:50

@alexmiller I'm aware of that, but my example above seems to work in Clojure

anmonteiro23:01:21

it works as if matching a literal vector

anmonteiro23:01:36

my understanding is that ClojureScript should also mimic that behavior

Alex Miller (Clojure team)23:01:03

I'm saying I think maybe Clojure is wrong here

Alex Miller (Clojure team)23:01:27

I would need to do some more poking to be more sure

anmonteiro23:01:48

right. so maybe needs some more analysis from your part before we reach any conclusions