Fork me on GitHub
#cljs-dev
<
2018-03-29
>
dnolen18:03:35

thanks applying and testing

richiardiandrea18:03:38

Question. Does hinting on return object works currently?

(defn my-fn [x] ^js/Foo.Bar
  ...)

dnolen19:03:22

you need to move the type hint

richiardiandrea19:03:41

I mean, I see I have warnings here, but that's ok, I can open an issue in case

dnolen19:03:16

yes it should work

richiardiandrea19:03:12

actually I think my issue is a bit different, see

(defonce parent-logger
     (.createLogger bunyan #js {:name "ep-cloud" :serializers bunyan/stdSerializers}))
I need to type hint both after .createLogger and at the defonce level I guess

dnolen19:03:27

you should not

dnolen19:03:32

but how did you get bunyan?

richiardiandrea19:03:08

@dnolen how do you mean? It is in my :npm-deps

dnolen19:03:31

is this for client browser

dnolen19:03:36

if it is then you don’t need externs

dnolen19:03:51

then why do you need to bother with externs?

dnolen19:03:59

advanced compilation is not meaningful

richiardiandrea19:03:23

well, I am experimenting, my payload will go in a lambda, so size might matter

dnolen19:03:51

doesn’t seem meaningful to me

dnolen19:03:07

Node.js dep graphs are already ridiculous

dnolen19:03:14

advanced compilation isn’t getting you anything

dnolen19:03:48

:simple will suffice

dnolen19:03:03

put effort where it matters 🙂

richiardiandrea19:03:22

wise man lol 😄 I just wanted to play 🙂

john19:03:28

If you happen to be pushing cljs code between contexts that require serialization (granted, not a common use case), advance compiled can be way faster

dnolen19:03:53

I really don’t see how

dnolen19:03:59

I messed around with this a lot

john19:03:00

it's just less data

dnolen19:03:12

there’s just no evidence when I looked at this

dnolen19:03:16

parsing times don’t matter

john19:03:31

I don't think it'd matter in most node contexts

dnolen19:03:35

performance does not come from advanced compilation anyway

dnolen19:03:41

but our own optimizations

dnolen19:03:43

if you’re not targeting web clients advanced compilation is a waste of time IMO

richiardiandrea19:03:49

also, this performance gain would be only visible during cold startup

richiardiandrea19:03:13

but I wanted to see how far I could get, definitely not worth too much time on it

dnolen19:03:14

and only if parse time dominates, but I never saw a huge difference between simple and advanced

dnolen19:03:48

anyways my advice is to spend your time elsewhere

richiardiandrea19:03:37

yep will take your advice, seems though the return type hinting is not really effective in some case...

richiardiandrea19:03:54

will try to make a simpler repro if i can

dnolen19:03:57

just not interested in this class of problems

dnolen19:03:01

so don’t bother

dnolen19:03:09

if you’re doing stuff for the client then we care

richiardiandrea19:03:17

well you provide a feature, I was just giving feedback 😄

dnolen19:03:44

yes be we don’t want to spend effort on meaningless usecases

👍 4
dnolen19:03:26

if you’re trying this when targeting clients then those should become tickets

john20:03:03

@dnolen you want this added to the website?

[NOTE]
====
Note: When compiling in `:advanced` mode, ClojureScript source code
files will not be included in the `:output-to` directory. This is
so that source files are not inadvertently exposed in production.
====

dnolen20:03:30

no, because it’s not true

dnolen20:03:57

I do not see any reason to explain source maps anymore than we have

dnolen20:03:07

if anything we should link to outside docs about source maps

dnolen20:03:10

this is not our problem

john20:03:20

Sounds good. I may ask about some more help does on occasion. I'd like to try to capture some of the questions coming through the channels in our docs, when it makes sense.

dnolen20:03:25

I think a good rule of thumb is that we shouldn’t be replicating general web application stuff into our docs

👍 4
honzabrecka21:03:04

Let's say I want to provide a cljs library that internally depends on an npm package(s). With latest cljs consuming npm packages is like a breeze. One can either use :npm-deps map and :install-deps options and let the cljs compiler to install npm dependencies, or use well known tools in js world (npm, yarn) and install dependencies manually. The key is to provide node_modules directory. But this works on the project level only. How to note, describe, provide the information that the library depends on an npm package in the specific version range? The first option is better for my idea as it's standard way and :npm-deps map could be copied to deps.edn file. If so, the cljs compiler could gather these npm dependencies from project and all libraries, create a dependency graph (would be tricky, I know), install them and compile the project. But it has drawbacks such as npm outdated does not work. Second option uses the power of existing tooling, but it would require to include package.json file in jar. Then the same cljs compiler actions described in first option applies. General problem is that js can depend on multiple versions of one library at the same time; cljs can't as its dependencies are flat. I'd like to know your thoughts 🙂

dnolen21:03:42

Not interested in supporting version ranges and all that stuff

dnolen21:03:50

we already have support for upstream :npm-deps

honzabrecka21:03:58

Cool, I didn't know there's already support for upstream :npm-deps

dnolen21:03:32

if it’s in the deps.cljs in the JAR we use it same as externs etc.

dnolen21:03:01

we throw on conflicts, your compiler options must specify override

honzabrecka21:03:31

upstream npm-deps does not work for me - library (jar) contains deps.edn with {:npm-deps {"legendary-spoon" "0.0.1"}} and project depends on this library. Compilation fails, adding :install-deps true to project changes nothing

john22:03:38

@honzabrecka can you put together a minimal repro?

honzabrecka22:03:06

@john working on it

john22:03:24

K thanks. I'd like to try it over here

richiardiandrea22:03:22

@honzabrecka I think David was referring to deps.cljs and not deps.edn

honzabrecka22:03:13

last weeks there's deps.edn everywhere, so I totally overlooked that cljs extension 😄

👍 4
honzabrecka22:03:04

so it works of course 🙂

richiardiandrea22:03:01

so for transitive npm deps you need deps.cljs, maybe that file could be auto generated from :npm-deps? Sorry maybe I state the obvious here 😄

richiardiandrea22:03:08

ah ah! wow this reads so odd 🙂

honzabrecka22:03:48

as far as I observed the deps.cljs is not generated

richiardiandrea22:03:41

yep I suspected that

dnolen22:03:11

We’re not going to generate it