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?
@martinklepsch there is no breaking change, you should not declare two different dependencies that represent the namespace it's really that simple.
this could also be solved nicely for users if libs deploy two artifacts, one for npm one for cljsjs where the dependencies are different.
trying to do anything in ClojureScript about it is looking more and more like a kludge
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
bundle target didn't introduce this issue
@dnolen I don't think that's related to what Martin is asking.
Cljdoc is using the analyze-file api, and that's what @martinklepsch is asking about
@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
@dominicm re: patch, I just misunderstood what you were saying - patch welcome to fix this, it's pretty simple (or (current-state) (empty-state))
I think the docstrings could be updated if you like, to talk about the api helpers instead of the compiler details
@dnolen I'm happy to make the change that way. Apologies that I didn't convey this sufficiently.
not your fault, I was busy yesterday and just didn't read closely enough
@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
@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
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
Weekend is coming up :)
@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?
hrm I really don't know which is why I haven't commented on it
I think I need to reserve some time to consider that one - probably not for next week but shortly thereafter
Np. I will (try) and investigate. I'm quite anxious about breaking things in unusual circumstances I hadn't thought of.
yep
@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.
sure go for it
It's late, but shouldn't this be (:options @(ana-api/empty-state))
?
Can fix with the patch to make it look at current-state too
hrm? I'm confused by that
I don't think so
unless I'm missing something, (or (current-state) (empty-state opts))
if current-state
exists you can ignore opts
, it'll gets passed along anyhow
Blugh, sorry, my link went missing
https://github.com/clojure/clojurescript/blob/42bcb07b8bf23d57f98e4617e4c4c93347f09715/src/main/clojure/cljs/build/api.clj#L294 @dnolen
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
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
@dominicm yes please, that's wrong
Cool :) Np.
thanks