Fork me on GitHub
#cljs-dev
<
2017-05-17
>
thheller06:05:32

does someone know where the code is that decides that (ns my.app (:require [goog.something :as x])) (x/foo) does not emit the goog.something.foo.cljs$core$IFn$_invoke$arity$1 ? ... version but goes directly to goog.something.foo(...)? Is that just tied to goog?

thheller06:05:12

will trace back from comp/emit* :invoke but maybe someone knows already

thheller09:05:46

as assumed goog is hard coded, created issue here https://dev.clojure.org/jira/browse/CLJS-2040

thheller17:05:41

looking for feedback from folks writing node apps about a new feature I'm working on, see https://github.com/thheller/shadow-cljs/issues/10

thheller17:05:31

do you think this would be an improvement to the current js/require situation or is this not a problem at all?

anmonteiro17:05:45

@thheller do you have any proposal for require('react-dom/server') style things?

thheller17:05:55

already works

thheller17:05:13

(:require [npm.react-dom.server :as rdom])

thheller17:05:08

basically every dot is converted to a slash and thats it 😉

anmonteiro17:05:35

I’ve been thinking that how we should support it in :npm-deps in the CLJS compiler proper

thheller17:05:46

(:import [npm.react-native Text View]) etc

juhoteperi17:05:04

What is someone has namespace starting with npm.?

thheller17:05:06

@anmonteiro yeah I figured you were working on something similar, thats why I started writing things down 🙂

anmonteiro17:05:30

“working” is not really the word yet

thheller17:05:31

@juhoteperi its a special reserved alias, it would complain

thheller17:05:04

we could use a special character to guard against that

thheller17:05:14

(:import [npm$react-native Text View]) or some other character that doesn't break the reader

thheller17:05:06

or (:import [^:npm react-native Text View])

juhoteperi17:05:26

Well, the naming is something to think about but probably npm. prefix would be just fine

juhoteperi17:05:46

@anmonteiro Doesn't module processing already allow using :require with the name used in :foreing-libs entry?

anmonteiro17:05:13

yes, but it doesn’t know about slashes yet

juhoteperi17:05:12

and node-inputs expands foreign-libs entries so that instead of directories it contains list all the JS files from npm modules?

anmonteiro17:05:51

based on what you require

anmonteiro17:05:08

it doesn’t expand blindly, it calculates the dependency graph

juhoteperi17:05:01

wouldn't that generate :provides ["react-dom"] for react-dom npm package index file, and :provides ["react-dom.server"] for server.js file in react-dom package?

juhoteperi17:05:52

I tested something like that at least, not sure what the current implementation is

anmonteiro17:05:22

I don’t think it currently does that

juhoteperi17:05:51

Right, if I read this correctly provides is only added to main files: https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/module_deps.js

juhoteperi17:05:46

Should be easy to generate provides for any files by relativizing the path against :roots or what ever the package.json option is and replacing / with .

juhoteperi17:05:29

Without prefix there is greater chance the there will be name clashes, but I guess :provides values could be prefixed if wanted

thheller17:05:59

How does the :npm-deps stuff handle the duplication of npm packages? does it dedupe or just include the code twice?

darwin18:05:49

@thheller npm may not be around in few years, yarn or something else could replace it, I would suggest more general mechanism of requiring “foreign” code via native package system - we have :require-macros we could have new :require-native or :require-foreign or something like that, without introducing artificial namespaces aka magic

richiardiandrea18:05:22

darwin: all for this, we should avoid hardcoding names

thheller18:05:57

@darwin good point on the magic part, I kinda agree

darwin18:05:46

I personally would vote for something like this (:require-native [react-dom.server :as rdom :via npm]), where :via would be optional, by default the compiler would try to figure out which native package system to use

darwin18:05:14

just my $0.02 - didn’t give this much thought

thheller18:05:57

I think we should then stick to what require does and not try to translate the string to a symbol

thheller18:05:18

(:require-native ["react-dom/server" :as rdom])

juhoteperi18:05:37

What about just using :foreign-libs to map from a name to a file?

thheller18:05:45

(:require-native ["../../foo" :as rdom])

thheller18:05:34

@juhoteperi I want to get rid of :foreign-libs if possible so not a fan of that idea

juhoteperi18:05:21

Users shouldn't not be required to write it, but I find it works well with :npm-deps as API between npm integration and Cljs compiler

thheller18:05:37

maybe (:js/require ["../../foo" :as foo]) so it mirrors the js/require form just like :require has a require mirror?

thheller18:05:24

@juhoteperi yeah I'm still not a fan of :npm-deps either, much rather leave that to yarn or npm

thheller18:05:15

the compiler could just read the package.json then

favila18:05:27

isn't npm packaging the maven of js? will there really ever be anything else? (Not talking about the tool, talking about package.json, the npm repository layout, etc) @darwin

darwin18:05:09

I don’t use “maven” anywhere in my code, so I don’t really want to mention “npm” there, that is just underlying impl detail, IMO

dnolen18:05:32

neither Maven nor NPM are going anywhere

darwin18:05:46

my idea with :via npm was bad, this should be resolved at project.clj/boot level

dnolen18:05:11

and complicating the ns form for this isn’t going to happen

dnolen18:05:54

@thheller there’s a lot of stuff we don’t handle - enhancements will be made based on feedback on actual :npm-deps usage

thheller18:05:07

@dnolen I didn't like :npm-deps to begin with so I'm obviously exploring alternatives ...

dnolen18:05:32

@thheller yes I know - but I’m not really concerned about that 🙂

thheller18:05:05

not expecting you to be ... but I think you were too fast to accept :npm-deps and when I can show something better I hope you'll reconsider. that is if I actually find something better. 😉

richiardiandrea18:05:03

Some feedback: I was working on Reason and BuckleScript a while ago and they have their own bsconfig.json file. This means you have to add the dependency twice. No good, hopefully Cljs will be able to avoid that.

dnolen18:05:06

I’m not holding my breath, since I don’t have any problems with :npm-deps at all

thheller18:05:14

just like :foreign-libs has a ton of issues that need solving

dvingo19:05:20

There are well documented flaws with the default npm client: https://code.facebook.com/posts/1840075619545360

dvingo19:05:57

The most serious around security in that you always have to go over a network

dvingo19:05:35

Given that open extension is also a core value of clojure it is also odd to tie this feature to a concrete implementation (npm) with no way of changing it.

dnolen19:05:03

I don’t really understand this line of reasoning

dnolen19:05:11

if you don’t want to use :npm-deps … don’t use it

dvingo19:05:36

Right, I don't plan on using it. The line of reasoning is that it isn't useful and is a waste of resources in dev time.

dvingo19:05:50

Why develop something that isn't going to be used?

thheller19:05:19

@dnolen you might still want to use a library that is using it though. so you may be exposed to it whether you want to or not.

dvingo19:05:22

The JS community is quickly moving off npm

thheller19:05:53

just like "everything" requires cljsjs.react now and people sometimes have to work around that

thheller19:05:40

case in point https://github.com/cljsjs/packages/tree/master/material-ui with the :exclusions. that is not user friendly and just gets worse the more foreign packages you use.

thheller19:05:02

or where you have to create empty (ns cljsjs.react) files if React is not provided by :foreign-libs

dnolen19:05:06

@thheller it’s called managing dependencies

dnolen19:05:25

I don’t see how this any different from any audit one must do

dnolen19:05:23

@danvingo if you can point at some kind of survey that shows a mass exodus off NPM as dependency delivery mechanism I will look at it

dvingo19:05:33

my evidence is anecdotal, from blog posts and repos I use, I understand that was a non-evidence based statement.

dnolen19:05:47

right, then I am unperturbed 🙂

dnolen19:05:30

@thheller if people want to use complex JS packages that don’t play well with Google Closure - they know what they are getting into

thheller19:05:31

@dnolen that is not managing dependencies ... that is working around issues that don't need to exist in the first place.

dnolen19:05:12

@thheller sorry I don’t really have anything more to add, and I don’t find your positions convincing … yet

thheller19:05:42

no worries ... I'm just exploring and looking for feedback. absolutely no expectations beyond that.

thheller19:05:10

just deleted the npm.react alias code based on the feedback although it was already fully implemented

dnolen19:05:20

@anmonteiro fwiw, I honestly don’t have a problem with supporting strings in :require instead of just symbols

dnolen19:05:36

nobody will ever use this feature for anything but loading random npm modules anyhow

mfikes21:05:00

If you have foo/core.clj:

(ns foo.core)

(defmacro add [a b] `(+ ~a ~b))

(defmacro funky [form] (eval form))

mfikes21:05:25

In Clojure (funky (add 2 3)) works only if you refer both funky and add (leaving off add results in the inner eval failing to resolve add). In ClojureScript, a refer for add is insufficient.

mfikes21:05:56

This is more of a curiosity on my part (but perhaps it could be a corner case in that the ClojureScript compiler could inject more into env when doing macroexpansion). Curious if there is any clear view on this corner case.

mfikes21:05:58

This came up in the context of this SO http://stackoverflow.com/questions/43985923/how-can-i-force-evaluation-of-nested-macros-in-clojurescript For me it is an academic interest, but it got me thinking that there might be a bug. Anyway asking mainly out of the academic interest. (It crossed into an area where my mental model is weaker than I'd like it to be.)