Fork me on GitHub
#cljs-dev
<
2017-08-18
>
anmonteiro04:08:55

cc @mhuebert if you wanna try it locally

anmonteiro05:08:12

also fixed https://dev.clojure.org/jira/browse/CLJS-2326 just now. @dnolen one of these will probably need a rebase once you apply the first one

anmonteiro05:08:20

let me know if and which one

mhuebert12:08:49

@anmonteiro appears to be working! able to refer react/react-dom, and also this is the first time I’ve got a :modules build working (previously had some issues with some transitive js-deps which have been sorted out in your recent work)

dnolen12:08:16

it’s also working for me here

dnolen13:08:43

Alex Miller’s clj stuff is really quite nice, I’m thinking we should switch our primary docs to it when it’s properly released

dnolen13:08:16

we don’t have to talk about rlwrap anymore, and it’s way simpler to cover deps than project.clj or Maven

mfikes13:08:35

It makes you wonder if it could lead to an alternative to the cljs.jar approach

dnolen13:08:53

yes IMO it really solves that problem

mfikes13:08:04

(Meaning, cljs.jar seems to bundle a lot of deps on the user’s behalf)

dnolen13:08:25

clj would be an acceptable way to submit minimal repros

mfikes13:08:57

Longer term, I wonder if some of the same magic that clj uses to search for the “latest” release could somehow lead to something similar that searches for the latest ClojureScript release. (It’s perhaps way to early to get a feel for what something like that might mean.) But, yeah, just using clj as it is meant to be used seems like a good idea.

juhoteperi13:08:11

@anmonteiro Do you have any tips on getting IntelliJ to detect Closure-compiler properly? Looks like it only supports pom.xml file and I think Closure has proper configuration on pom-main.xml file

mhuebert14:08:22

is the clj stuff online somewhere? nvrmind: found the thread https://groups.google.com/forum/#!topic/clojure/FpMqtNu0TCE

mhuebert14:08:25

I’m compiling using :optimizations :simple for the first time now with the latest cljs, and I am getting a runtime error that I haven’t seen before: Uncaught TypeError: Cannot read property 'env' of undefined. There is just the one file in the stacktrace (the :simple-compiled file). The error comes from fbjs in node_modules, trying to read process.env: var module$Users$MattPro$Documents$sites2017$maria$node_modules$fbjs$lib$emptyObject={};"production"!==process.env.NODE_ENV&&Object.freeze(module$Users$MattPro$Documents$sites2017$maria$node_modules$fbjs$lib$emptyObject);function makeEmptyFunction...

mhuebert14:08:13

I can avoid the error by running <script type="text/javascript">window.process = {env: {}}</script> first

dnolen15:08:33

@mhuebert I suspect this might be a dep sorting issue due to :preloads

dnolen15:08:56

turn off optimization set :debug-inputs true and paste the dumped inputs somewhere

dnolen15:08:11

in the meantime your workaround is probably the right way to go

dnolen15:08:40

@tony.kay when you get a chance could you try master as well - I tweaked how we handle :preloads, in your case I want to make sure your :modules stuff is still working wrt. dep order when applying optimization settings

richiardiandrea15:08:35

I have noticed this little quirk while trying the string require, which is a bit odd (at the REPL):

(require '"something") ;; there is a quote before the apices
The why is clear, but it feels unidiomatic, should I open an issue or not worth fixing?

dnolen15:08:29

minor ticket is fine

anmonteiro15:08:22

@juhoteperi sorry I dont use intellij

dnolen15:08:03

@juhoteperi IntelliJ processes all the poms

dnolen15:08:09

they become different Maven profiles in the project

juhoteperi15:08:25

Oh, I updated IntelliJ and now I can see the profiles

richiardiandrea16:08:08

I have a question, with the advent of the new string require, is there a more concise way to require https://google.github.io/closure-library/api/goog.string.format.html ?

richiardiandrea16:08:34

sorry maybe I am off-topic here actually

anmonteiro16:08:34

@richiardiandrea no. different concerns

anmonteiro17:08:21

I didn’t try material after putting it together though

anmonteiro17:08:08

@dnolen if you’re still around, another enhancement patch: https://dev.clojure.org/jira/browse/CLJS-2330

dnolen18:08:31

@juhoteperi we could maybe use global exports in node_modules case to fix old style usage

anmonteiro18:08:49

@dnolen this is only for compatibility & consistency with a case that we don’t really expose, but advanced users can poke at

dnolen18:08:50

since global exports tells us the globals, we could rewrite those references

anmonteiro18:08:59

(I’m speaking about CLJS-2330)

anmonteiro18:08:28

because processing node_modules under :target :nodejs doesn’t really work with :none, but you can use that with :simple

anmonteiro18:08:43

if you’re really determined

juhoteperi18:08:24

@dnolen Hmm, interesting. Even (def a js/React) (.createElement a ...) should probably work as a would point to module$foo$bar$react or something, and Closure should take care of renaming the calls?

dnolen18:08:53

@juhoteperi what I’m suggesting is this

dnolen18:08:14

if we have global exports defined they are applied universally

dnolen18:08:25

js/React can never be js/React

dnolen18:08:43

it will always get rewritten to a generated alias

dnolen18:08:12

(:require [cljsjs.react]) becomes (:require [cljsjs.react :as module$React])

dnolen18:08:42

then (def a js/React.Component) -> (def a module$React/Component)

dnolen18:08:01

this would should fix all old style usage as long as people aren’t doing crazy dynamic stuff - in which case don’t really care

juhoteperi18:08:36

What do you mean with crazy dynamic stuff?

juhoteperi18:08:53

Something like (def react (cond (exists js/React) js/React ...))

dnolen18:08:05

no, I mean stuff like (goog.object/get js/window "React")

juhoteperi18:08:20

Okay, I think no major libs do that

juhoteperi18:08:09

Anyway, I already updated to om/rum/devcards/sablono to use new require, but couldn't get tests passing with simple changes. I'll put up PR on each project for commenting and testing.

juhoteperi18:08:56

And I'll link to that

dnolen18:08:40

cool, thanks

juhoteperi18:08:17

I think most of the problems with the changes are from code generated from macros that tries to call react/

juhoteperi18:08:19

E.g. (.apply react/Component this# (js-arguments)) at om.next defui*

juhoteperi18:08:31

Causes Uncaught TypeError: Cannot read property 'Component' of undefined

dnolen18:08:11

hrm ok, I’ll have to look at that later

juhoteperi18:08:49

There is also lots of warnings from namespaces that use defui about missing react namespace

juhoteperi18:08:08

Those namespaces don't use react/ directly so they don't have require

bhauman18:08:09

@juhoteperi I've been reading the comments and trying to understand the need for this change, and to know how backwards compatible it is

juhoteperi18:08:30

It will allow users to use the library with both foreign-libs and node modules. Which is not really possible (currently) if library accessess js/React or other globals.

dnolen18:08:13

@bhauman short summary - it doesn’t really affect current usage

dnolen18:08:28

however making old stuff and new stuff work at the same time will require a bit more work

bhauman18:08:50

thanks guys

dnolen18:08:01

the important thing to understand is that :global-exports doesn’t break js/Foo

dnolen18:08:15

it just allows you to use foreign libs like normal requires

dnolen18:08:45

CLJS-2331 would go the extra mile

dnolen18:08:01

it would convert all old style foreign libs usage to become new style foreign lib usage

tony.kay18:08:39

@dnolen Yeah, but it’ll be tomorrow. Busy today.

bhauman18:08:27

Very very cool stuff, best of all worlds. Thats the way I like my cake.