Fork me on GitHub
#cljs-dev
<
2020-06-12
>
martinklepsch06:06:28

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?

dnolen12:06:47

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

dnolen12:06:32

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

dnolen12:06:09

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

dnolen12:06:55

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

dnolen12:06:12

bundle target didn't introduce this issue

dominicm12:06:29

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

dominicm12:06:23

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

dnolen13:06:04

@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

dnolen13:06:53

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

dnolen13:06:47

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

dominicm13:06:58

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

dnolen13:06:04

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

dnolen13:06:07

@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

dnolen14:06:08

@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

dnolen14:06:40

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

dominicm14:06:50

Weekend is coming up :)

dominicm14:06:40

@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?

dnolen15:06:11

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

dnolen15:06:23

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

dominicm15:06:26

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

dominicm15:06:26

@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.

dnolen15:06:10

sure go for it

dominicm21:06:10

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

dominicm21:06:19

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

dnolen21:06:16

hrm? I'm confused by that

dnolen21:06:10

I don't think so

dnolen21:06:39

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

dnolen21:06:02

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

dominicm21:06:36

Blugh, sorry, my link went missing

dnolen21:06:23

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

dnolen21:06:52

@dominicm yes please, that's wrong

dominicm21:06:57

Cool :) Np.