Clojurians Log v2

@bhauman hrm, I can't reproduce https://clojure.atlassian.net/browse/CLJS-3258 with either master or 773

maybe you should add -Srepro to your clj command?

I couldn't reproduce this one either https://clojure.atlassian.net/browse/CLJS-3253, I made something minimal no warnings for me

both of seem like issues that were fixed sometime ago - like you're not running the version of ClojureScript you think you are - but that may be a bad hunch

@dominicm I don't think you added whether this ticket was a regression or not https://clojure.atlassian.net/browse/CLJS-3255?

@dnolen hmmm thats really strange about https://clojure.atlassian.net/browse/CLJS-3258 , as it was reported to me, and then I reproduced it twice in 773 in two separate builds

did you look at the compiler output?

yep

but you know what it might be cache bug?

delete ~/.cljs

and try again

hmmmmmmm

ah yes

probably that but please confirm

it's because :target will always be :nodejs

there's nothing to differentiate

@dnolen every version I checked was broken, I got a different error at some point. I think it was arity, but not sure.

@dominicm right just not supported ok, going to clarify

Fwiw, it works for advanced, but not dev.

@dnolen oh dammit

yeah it works, and its using the global “React”

k

so its a cache bug

well that’s really go to know

I thought I tried :aot-cache false in my testing but oh well

@bhauman try master

cool will do

@dnolen I have to populate the cache with something bad first right?

How do I get to that place?

no

to test

just switch between :target :bundle and :target :nodejs

it should always work

gotcha

I mean you can try to run it but it's really not necessary

just check the output

ok it works

actually not

the bug occurs when you change from one kind of dep to another

I.E. when you are shifting between cljsjs libs to node libs

ah that’s hard

and it will be common when folks are porting their apps

the cached version of reagent will be depending on the global react

or vice versa

so I can reproduce this by switching the type (cljsjs or node) of dependency for a given library

@bhauman master should address this problem - look at the output

:nodejs-rt should be in the first line now where it wasn't before

I saw that

but when you switch everything gets recompiled

oh

yep

but maybe cache-keys lemme check

@bhauman nope - it should part of the cache key now

so I do not understand what you are saying yet

there's no such thing as "cached reagent"

OK if I have react installed node modules

that's not cached according to all build affecting options

and the I remove react from node_modules

so this isn’t reflected in the build options

I’m not changing build options

its affected by the presence or absence of a node_module

in one case reagent is compiled referencing a node_module, and in another it is compiled referencing a global React

so it gets cached one way or the other

even though they both have nodejs-rt false

the waters are getting muddied right now by this description, let describe how I think it can actually go wrong.

here's what I think will happen normally

Scenario A: User has built Reagent w/ React CLJSJS w/o :target :bundle

1. The switch to :target :bundle (and installed React into node_modules everything will be recompiled everything will be cached on a new path

@bhauman ^ do you agree this is a happy path for most people?

I’m trying to think of the best way to communicate this

let's just go w/ what I'm saying

is this the happy path - yes or no?

then we can describe things that could wrong because of missed steps

I think the normal case is that they switch to bundle and build

without adding the node_modules

I'm just trying to describe a minimal reproducer, let's just ignore all the things that can go wrong

oh

I understand you're interested in the UX but we only need a reproducer

cool

ok - so the above scenario works

2. what happens is that if the user never runs the install step - they will cache something that points to CLJSJS

I’m actually having to move a TV, I’ll be back

k!

@dnolen OK back

sorry bout that, ping me when you’re ready 🙂

ready

do you agree about 2. ?

yes that can happen 🙂

do you agree that all other cases are really some form of 2. ?

yes caching something under :nodejs-rt false

the order may change what gets cached - but in the end it doesn't really matter, they all look like 2.

or just caching something

yes

well they can cache something that points to CLJSJS or a node modules require

that’s a more general 2

more specifically a library that exists outside of the project like reagent gets cached that way

so here's the thing - I don't actually see any good answers to this problem yet - let's point out some basic assumptions

1. Realated, AOT Cache can't understand macro time configuration because it is file specific, not build specific

2. AOT Cache caches along build configuration computed path only, no file specific stuff as stated in 1.

I understand that

3. Whether we use node module or CLJSJS is in fact exactly the same same problem as 1., a file specific thing

yep

4. it's allowed that what's in node_modules is never specified

cool you understand the problem then

I totally get that it may not be fixable

well it points a bunch of prior choices that make it appear incompatible with a simple caching strategy

one alternative is validation to prevent cache corruption

i.e. break 4. or at least supply a flag to break 4.

that is you can't use something not both in :npm-deps and in node_modules

doesn’t sound ideal either

in practice there is a rough patch when folks are switching over

let's be clear to

this only affects any tool default to :aot-cache true

which is not true by default

and are unaware of what libraries depend on what

you don't need to know what depends on what

upstream :npm-deps is a thing

cljs.main defaults to :aot-cache true

no burden on the user other than installing the deps and specifying their own

yes cljs.main specifically designed for people who are going to audit their deps because we don't care about the macro build time problem

so we're already just saying you're on own - I don't really care that much about this caching problem and order stuff because it's a one time thing

but I do care that downstream tooling can provide better UX if that's desired

but breaking 4. is not as bad it sounds - it's more disciplined and would in fact catch simple mistakes

really 4 is just :enforce-deps - either :install-deps is true, or the libs are present at the top level

this means it's not possible to use Reagent w/o installing the deps first - because you're saying they must come from there not elsewhere

since node_modules wins over foreign libs

just noting that changing caching is quite unattractive because you would be generated useless cache files while you're changing your deps at the beginning of the transition

better to just turn it off during transition

the cache is not build dependent so this gets tricky

yes it doesn't know anything about builds - just build affecting options

because you want the cache to be available to all builds

again this is due to dependency ambiguity

which isn't generally true for Clojure(Script) stuff, just libs that don't enforce one or the other

so this once again the same problem rearing it's head

buts its only because the way of requiring the dependency is in the file itself

no

it its not abstracted

it has nothing to do with that

it's a dependency ambiguity plain and simple

OK bear with me

yes I hear that

you don't know know where the dependency is coming from

hold on

if the code for requiring the cljsjs dependency and the node dependency was rendered the same in the output this isn’t a problem

completely unrelated

think about a two Maven dependencies with the same package name

you don't know what you're going to cache

this happens with Clojure AOT

I’m saying that if this line

reagent.impl.component.global$module$react = goog.global["React"];

was the same for both node and cljsjs

then they are caching the same thing

I'm just pointing out you're talking about solutions

yes

i get that you are addressing fundamentals

yes

to your point - I just don't see anything fruitful around how we "link" the library

but maybe you see something, all ears

having external libs be linked in a similar manner is kinda interesting to me

sure but I don't see anything that would work?

well under bundle we have npm_deps, why not have cljsjs libs scoped to a JS object?

it kinda sucks that these things are global anyway

there are problems with backward compatibility here, which is a real practical problem

also load order w/ webpack

we are async webpack isn't

also maybe a node_module DCE issue?

anyways this would be moving around a lot of chairs

yeah nothing simple

so it's not very enticing

we couldn’t have the npm_deps.js inline the cljsjs lib could we?

nah load order

never mind

welll

I think maybe load order isn't an issue

because webpack ensures everything is present - this works for the REPL

yeah yeah

but for DCE and code splitting ... changing stuff is probably a bad idea

you really don't want foreign libs to go into one place for code splitting

for :bundle npm_deps.js is dev time only, so the door remains open for DCE improvements later

so there's just a bunch of considerations in the way of making this easy

only to make everything else hard

good points

one other option probably has it's own issues is emitting require for foreign libs

but then you need to configure your bundler so that's moving work around

I think the issue need more stewing

hrm though webpack can handle relative require so maybe no config

I think the blocker here is that many foreign libs are probably packaged expecting to be global?

fixing up a bunch of libraries when you're transitioning away doesn't sound very valuable

@juhoteperi I think worth pondering dropping the Reagent explicit dependency on cljsjs/react users can always add it

I think pointing to both things creates more problems than it's worth as the backlog will show

@bhauman I think the simplest answer is to just have libraries just choose one dependency

then the problem just goes away

@dnolen Yes, I've been planning to drop the cljsjs dependency

Probably before final 1.0 release

k