Fork me on GitHub
#cljs-dev
<
2017-03-27
>
anmonteiro02:03:28

@martinklepsch looked at your mobiledoc-kit issue and it looks to me like a Closure Compiler bug at this point. Maybe something related to module ordering

anmonteiro02:03:14

what's weird to me is that it says module$....$node_modules$mobiledoc_kit$dist$commonjs$mobiledoc_kit$utils$cursor$position is not defined

anmonteiro02:03:26

but if you type that in the dev tools console it's there

anmonteiro02:03:08

which makes me think it must be loaded after it's required initially

anmonteiro02:03:38

in fact, if you try to compile with optimizations, it will tell you exactly that:

SEVERE: /Users/anmonteiro/Desktop/modules-playground/out/node_modules/mobiledoc-kit/dist/commonjs/mobiledoc-kit/utils/cursor/position.js:2: ERROR - required "module$Users$anmonteiro$Desktop$modules-playground$node_modules$mobiledoc_kit$dist$commonjs$mobiledoc_kit$utils$cursor$range" namespace not provided yet

martinklepsch05:03:00

> but if you type that in the dev tools console it's there yeah that confused me too

martinklepsch05:03:39

@anmonteiro thanks for looking into that. I’ll try to find out if this is a result of the cyclic requires in those namespaces, seems most likely at the moment… Do you know if there are any restrictions in Closure’s module system with regards to that?

anmonteiro14:03:33

@martinklepsch that shouldn't matter, I think

anmonteiro14:03:56

At least it doesnt in JS, because module requires are cached

favila16:03:38

Found interesting things looking at bool type hinting in the generated js

favila16:03:08

Particuarly struck that even when the (and...) expression in the fn body is known to be bool, yet the analyzer doesn't seem to see that. An outer if still uses truth_()

favila16:03:28

I thought explicit ^boolean hint was unnecessary in those cases

dnolen17:03:38

@favila hrm might be regression

dnolen17:03:24

my hunch is that’s it not about boolean type hint

dnolen17:03:44

so much as the compiler determining some other tag vs. explicit one overriding any inferred thing

dnolen17:03:12

we should be doing the later - but maybe my tweaks for JS externs inference changed something

anmonteiro17:03:02

@dnolen that would make sense now that you say it. @darwin mentioned some time ago that the externs inference thing broke something in devtools which was fixed by an explicit boolean type hint

anmonteiro17:03:05

something like that

darwin17:03:59

These days I tend to use macros to emit calls to code which is expected to be (optionally) DCE-ed

darwin17:03:31

I think unexpected prevention of DCE is more general problem, once I hit a problem that only mentioning a function in a static config map literal prevented DCE, although the function was never called - closure compiler didn’t see that, e.g. here: https://github.com/binaryage/chromex/blob/fdd491d1971b152d2d20209a696f15649e10c12a/src/lib/chromex/config.clj#L53-L55

anmonteiro17:03:58

@dnolen also have another question: yesterday I stumbled upon a piece of code that I thought would work but didn’t if you self-`require-macros`, should you expect the macros of that namespace to be automatically referred?

anmonteiro17:03:49

is that because of some technical limitation?

dnolen17:03:52

I mean you could expect that to work

dnolen17:03:57

but there’s nothing there to make it work

anmonteiro17:03:11

it would seem to me that that would be the next step of the automatic inference stuff

dnolen17:03:19

no technical limitation that I can think of no

anmonteiro17:03:30

should I make a ticket for it?

dnolen17:03:42

hrm I don’t know ...

dnolen17:03:49

it introduces a new kind of clobbering

anmonteiro17:03:46

@dnolen say you have a foo.core namespace in Clojure. You’d expect macros you define there to be referred automatically

anmonteiro17:03:53

because it’s the same namespace

anmonteiro17:03:12

the fact that in CLJS it happens in a different compilation step shouldn’t change that, IMO

dnolen17:03:01

well in ClojureScript it’s not really the same namespace

dnolen17:03:10

there’s 2 namespaces in ClojureScript, one for runtime one for macros

dnolen17:03:31

I’m more concerned about whether doing this automatically is going to break existing programs in weird ways

dnolen17:03:40

clobbering

anmonteiro17:03:37

I don’t really understand the implications

anmonteiro17:03:13

I suppose we could try it out and run it in a few well-known libraries

anmonteiro17:03:40

not high priority anyway

anmonteiro17:03:49

just something that I expected to work and didn't

dnolen17:03:57

@anmonteiro what will happen to existing programs that :require-macro once auto refer like you suggest works?

dnolen17:03:07

if the macro ns and the runtime ns use the same name

anmonteiro17:03:35

I was only suggesting it for NSes that use the same name

dnolen17:03:52

the same var name

anmonteiro17:03:56

the only problem I foresee would be shadowing vars

dnolen17:03:03

it won’t shadow

dnolen17:03:06

it will do something different

anmonteiro17:03:40

so I guess one example would be cljs.core, right?

anmonteiro17:03:50

you have or which is both a macro and a function

dnolen17:03:08

yes anybody that is doing that will suddenly have a different program

anmonteiro17:03:31

will they though?

anmonteiro17:03:57

I don’t see how

dnolen17:03:00

only if they were careful to make sure the macro and runtime thing are actually same

dnolen17:03:35

but really this is problem way beyond libraries

anmonteiro17:03:45

@dnolen how is that different from what macro inference does today? (require [foo.core :refer [or]])

dnolen17:03:45

you don’t know what kind of naming conventions people are using internally

dnolen17:03:57

@anmonteiro it’s completely different

dnolen17:03:01

because you had to buy in first

dnolen17:03:16

what you’re talking about sucks people into a behavior they didn’t ever express

dnolen17:03:36

so now you have all this code that assumes one thing

anmonteiro17:03:46

I’m just talking about making this work if you require-macros of a namespace with the same name

dnolen17:03:47

but once you change the semantics, it means that code may or may not be valid

anmonteiro17:03:50

isn’t that also buying in?

dnolen18:03:01

because maybe already have code that did that

dnolen18:03:33

for the other feature, almost no one had any existing code that worked that way

dnolen18:03:55

everybody wrote separate macros nses following the lead of the ClojureScript libraries we were releasing

dnolen18:03:07

what you’re suggesting will definitely affect code right now - since they are definitely require-macros and using the same ns names

anmonteiro18:03:49

I know I can’t know what naming conventions people use for their own stuff.. But I still don’t predict this would have such an impact as you’re saying. I’m probably still missing something

anmonteiro18:03:06

like, if we auto-refer those macros, doesn’t it only affect that same namespace?

anmonteiro18:03:46

and I would say most people are referring those macros anyway, just like we do here https://github.com/omcljs/om/blob/master/src/main/om/next.cljc#L4

dnolen18:03:20

I agree that it might not be a problem - but it’s probably not worth doing because we don’t know and it will affect many programs

anmonteiro18:03:32

yeah I’m fine about the way it works today. Just thought it would be interesting to bring up, my point of view is it would be another step in bridging the gap between Clojure and ClojureScript - and I like those kinds of enhancements 🙂

dnolen18:03:10

sure, but I don’t like changes where we obviously can’t know all the downstream effects 🙂

dnolen18:03:27

plenty of innocuous looking things get in as it is and these cause problems

dnolen18:03:22

a more conservative approach would be compiler flag

anmonteiro18:03:02

Wrt compiler flags I kinda feel we have too many of those already

dnolen18:03:41

I don’t have a problem with compiler flags

anmonteiro18:03:48

but we could always add one for people to test it in their own projects and remove it if it turns out to work?

dnolen18:03:53

if something proves itself to be good after 6 months then we can make it the default

anmonteiro18:03:03

or not remove it, just make it the default

dnolen18:03:04

many compiler features started this way

dnolen18:03:25

analysis caching, fn optimization etc.

anmonteiro18:03:27

I see. that sounds like something to think about

anmonteiro18:03:45

that could also bear fruits without impacting stability

anmonteiro18:03:23

@dnolen btw I don’t think I ever reported back, but sometime ago I started working on warning about private var usages, and I feel like we can never introduce that

anmonteiro18:03:49

or it would also have to be something we introduce behind a flag so that people can incrementally migrate

anmonteiro18:03:11

it would also be a huge patch as we use so many private things internally as well 🙂

anmonteiro18:03:46

anyway, probably time to get back to work, thanks for the discussion

dnolen18:03:42

@anmonteiro pretty much why we still don’t have that 🙂