cljs-dev

ClojureScript compiler & std lib dev, https://clojurescript.org/community/dev
martinklepsch 2020-06-12T06:43:28.256800Z

Is there a way we could have seen this breaking change coming? I just checked the changelog and itโ€™s not mentioned. Or was our usage of this API incorrect in some way?

dnolen 2020-06-12T12:32:47.257700Z

@martinklepsch there is no breaking change, you should not declare two different dependencies that represent the namespace it's really that simple.

dnolen 2020-06-12T12:33:32.258400Z

this could also be solved nicely for users if libs deploy two artifacts, one for npm one for cljsjs where the dependencies are different.

dnolen 2020-06-12T12:34:09.258900Z

trying to do anything in ClojureScript about it is looking more and more like a kludge

dnolen 2020-06-12T12:49:55.259900Z

also as far as I can tell this is nothing new - was always an issue between AOT cache and flipping between foreign lib and node_modules

dnolen 2020-06-12T12:50:12.260300Z

bundle target didn't introduce this issue

dominicm 2020-06-12T12:57:29.261100Z

@dnolen I don't think that's related to what Martin is asking.

dominicm 2020-06-12T12:58:23.262200Z

Cljdoc is using the analyze-file api, and that's what @martinklepsch is asking about

dnolen 2020-06-12T13:13:04.265100Z

@dominicm @martinklepsch sorry I missed that, thanks. This is just an oversight on my part, because I have little visibility about api usage. We should probably figure out some way to test clj-doc, some of this would have to be in @alexmiller's domain if he wants to run stuff in Jenkins

dnolen 2020-06-12T13:13:53.266100Z

@dominicm re: patch, I just misunderstood what you were saying - patch welcome to fix this, it's pretty simple (or (current-state) (empty-state))

dnolen 2020-06-12T13:14:47.267700Z

I think the docstrings could be updated if you like, to talk about the api helpers instead of the compiler details

dominicm 2020-06-12T13:15:58.268600Z

@dnolen I'm happy to make the change that way. Apologies that I didn't convey this sufficiently.

dnolen 2020-06-12T13:19:04.269100Z

not your fault, I was busy yesterday and just didn't read closely enough

dnolen 2020-06-12T13:29:07.271300Z

@martinklepsch sorry about that, after talking a bit with @alexmiller - I'm going to look into using the GitHub Actions feature, first step is just getting CLJS tests running - testing stuff like cljs-doc is important so when we're there perhaps can chat about how that can be done

๐Ÿ‘ 1
dnolen 2020-06-12T14:03:08.272200Z

@dominicm happy to take that patch whenever you have it would like it to be in the next release along with a few other fixes for :bundle

dnolen 2020-06-12T14:03:40.272900Z

shooting for next Friday so there's some time, if you can't get to it not a big deal - I can get to it next week

dominicm 2020-06-12T14:15:50.273Z

Weekend is coming up :)

dominicm 2020-06-12T14:18:40.273200Z

@dnolen RE https://clojure.atlassian.net/projects/CLJS/issues/CLJS-3247 my real question over it is whether generating x["delete"] for js/x.delete seems OK. I'm worried about Google Closure. One scenario I invented up was that if someone was running their JS through Google Closure, then it might be broken. Do the tests cover that kind of case?

dnolen 2020-06-12T15:10:11.273600Z

hrm I really don't know which is why I haven't commented on it

dnolen 2020-06-12T15:11:23.274200Z

I think I need to reserve some time to consider that one - probably not for next week but shortly thereafter

dominicm 2020-06-12T15:11:26.274300Z

Np. I will (try) and investigate. I'm quite anxious about breaking things in unusual circumstances I hadn't thought of.

dnolen 2020-06-12T15:11:32.274500Z

yep

dominicm 2020-06-12T15:12:26.274600Z

@dnolen I have a rough patch I can upload if it would help explain what I'm doing / aid in others showing me how to test for it. I could probably use some feedback on how I've implemented it to - part of me worries it's not going to be a stylistic match for the project. It's a bit silly I guess.

dnolen 2020-06-12T15:47:10.274800Z

sure go for it

dominicm 2020-06-12T21:03:10.274900Z

It's late, but shouldn't this be (:options @(ana-api/empty-state))?

dominicm 2020-06-12T21:03:19.275Z

Can fix with the patch to make it look at current-state too

dnolen 2020-06-12T21:12:16.275200Z

hrm? I'm confused by that

dnolen 2020-06-12T21:13:10.275400Z

I don't think so

dnolen 2020-06-12T21:14:39.276600Z

unless I'm missing something, (or (current-state) (empty-state opts))

dnolen 2020-06-12T21:15:02.277100Z

if current-state exists you can ignore opts, it'll gets passed along anyhow

dominicm 2020-06-12T21:15:36.277300Z

Blugh, sorry, my link went missing

dnolen 2020-06-12T21:16:23.278300Z

heh, no worries, I'm about to head out for a bit but will check in later - I'll probably be looking into GH Actions for ClojureScript over the weekend

dominicm 2020-06-12T21:16:51.278500Z

Trying to point at the fact that it's using the state without deref'ing it. Contrast with https://github.com/clojure/clojurescript/blob/42bcb07b8bf23d57f98e4617e4c4c93347f09715/src/main/clojure/cljs/build/api.clj#L280

dnolen 2020-06-12T21:16:52.278600Z

@dominicm yes please, that's wrong

dominicm 2020-06-12T21:16:57.278900Z

Cool :) Np.

dnolen 2020-06-12T21:16:57.279Z

thanks