Fork me on GitHub
#clojurescript
<
2021-11-04
>
Chris McCormick07:11:22

Not sure if this is the right place to post this, but I am running into an issue running codox over a .cljc file with reader conditionals:

Could not generate clojure documentation for sitefox.deps - root cause: clojure.lang.Compiler$CompilerException Syntax error macroexpanding clojure.core/ns at (sitefox/deps.cljc:1:1).
Syntax error macroexpanding clojure.core/ns at (sitefox/deps.cljc:1:1).
Call to clojure.core/ns did not conform to spec.
...
Execution error (IllegalArgumentException) at codox.main/ns-matches? (main.clj:73).
no conversion to symbol
Any tips on how I can diagnose/fix this? The file is here for your reference: https://github.com/chr15m/sitefox/blob/main/src/sitefox/deps.cljc

dnolen11:11:45

@chris358 the namespace is a string

dnolen11:11:44

note this isn't necessary in ClojureScript since all those characters are valid in symbols

dnolen11:11:48

however note that shadow-cljs has non-standard semantics here which probably creates a lot of confusion - I'm not sure if nbb follows the standard behavior

thheller18:11:08

to clarify here: the only non-standard thing shadow-cljs is providing here is outputting ESM code. besides that it just provides "express" or whatever to import rather than require. The default mess is because of difference of the host runtime not because of anything shadow-cljs or nbb are doing.

thheller18:11:26

whether its "express$default" or express$default makes no semantic difference to shadow-cljs either

dnolen18:11:11

so you can require node modules now w/o strings? sorry I missed if that changed (or just misunderstood) - I thought it had to be a string to call it out

thheller19:11:06

you always could. I never diverged from the "standard" in that regard. I just personally recommend using strings to distinguish pure npm/JS requires from CLJS requires. personal subjective preference since strings need to exist anyways.

thheller19:11:21

I still absolutely do not like that the presence of a folder in node_modules decides whether a require is valid or not

thheller19:11:52

but that doesn't mean shadow-cljs won't follow the "standard"

dnolen19:11:54

got it sorry I thought that was how shadow actually worked

borkdude19:11:06

yes, #nbb follows only a standard (subset of) CLJS

Chris McCormick12:11:43

I have changed the requires to use the symbol style instead of strings but I am seeing the same error from codox. I've attached the traceback in case somebody has time to help me diagnose.

dnolen13:11:28

you need to show your altered ns form as well

borkdude13:11:17

@chris358 codox just needs to understand that you can use string namespaces in other CLJS dialects, or it should allow a set of cljc features to just skip those branches

Chris McCormick13:11:40

Do you think I should I take this to the codox issue tracker or is it something I can solve? Sorry I'm a bit out of my depth here. 😅

Chris McCormick13:11:04

I have removed the string namespaces and the error is still occurring.

borkdude13:11:06

It probably treats the :org.babashka/nbb as Clojure as opposed to CLJS. Or does it also complain if you get rid of the :org.babashka/nbb branches?

dnolen13:11:23

@chris358 this is yet another non-standard thing in shadow

dnolen13:11:42

custom reader conditionals - this cannot work if you're going to use this you should expect most tools to fail

dnolen13:11:54

Clojure doesn't not support them, probably won't thus ClojureScript does not

dnolen13:11:14

the fact that shadow does is not a good thing for a incredibly long list of reasons including the above

borkdude13:11:46

@U050B88UR That's actually not true. Clojure does support this. It's a property of clojure.tools.reader and also the Clojure reader that it just picks what it's interested in.

borkdude13:11:56

So it's not specific to shadow-cljs at all.

dnolen13:11:45

unless something changed - you are only allowed to use :clj :cljs or :cljr nothing else will work

dnolen13:11:44

it's literally the first thing called out

borkdude13:11:17

So CLJS barfs when you use a :random/dude reader conditional? Why the closed world assumption? Clojure itself supports it. Also see this README: https://clojure.org/reference/reader#_reader_conditionals > Implementors of non-official Clojure platforms should use a qualified keyword for their platform feature to avoid name collisions. Unqualified platform features are reserved for official platforms.

dnolen13:11:32

that's been changed since I last looked at it - been years - so the rules have been softened a bit

dnolen13:11:51

so where shadow takes the unqualified name is a problem

dnolen13:11:01

but not the qualified one

dnolen13:11:26

originally only the three above were allowed - I was actually there (at Cognitect) when it was decided upon because ClojureScript really needed it

dnolen13:11:52

but I guess w/ the ClojureDart etc. etc. this has opened up a bit

dnolen13:11:32

@chris358 you need to alter the reader somehow to codox to handle this stuff, not sure how that is done

Chris McCormick13:11:21

Ok thanks @U050B88UR I will file an issue on the codox repo and see if weavejester has time to look.

thheller16:11:05

> so where shadow takes the unqualified name is a problem

thheller16:11:22

shadow doesn't take a name at all. the user is free to specify whatever but it is up to the user. I just added it to solve a problem actual projects had

thheller16:11:50

all I did was make :reader-features configurable. there is nothing set or used by default that isn't already standard

dnolen18:11:16

Maybe should update the docs to avoid having users use non-namespaced names to specify platforms - other thing I wonder if we should suggest shared naming conventions i.e. :cljs/browser :cljs/node :cljs/react-native etc. perhaps it worth updating the docs to recommend the using namespaced keyword for custom usage - on a bigger note I wonder if we should recommend "standard" names, i.e.

dnolen18:11:33

then tooling can handle a common set

borkdude18:11:27

:cljs/es6 ;)

borkdude18:11:55

@U050B88UR why does CLJS expect a fixed set of reader features in .cljc files rather than an open set? I'm not sure if I get the issue. The namespaced ones are only for "non-official" (non-Cognitect?) Clojure implementations like :org.babashka/nbb - I'm actually the first one who conforms to this rule afaik. CLJS could of course add more :cljs/* ones for specific use cases.

borkdude18:11:41

Babashka still uses :bb as that addition to the docs was added after the fact.

borkdude18:11:04

I wasn't entirely joking about :cljs/es6, I've discussed this idea with Thomas once. This could be used to account for both CJS and ES6 targets in one file, since the surfacing libspec forms tends to be different in ES6 due to different export structure (the thing with default, etc).

Chris McCormick12:11:56

Ah ok thank you.

dnolen15:11:56

@suskeyhose re: bound-fn many years ago some options were considered but they were all quite complex w/ a lot of tradeoffs - in Clojure of course you have real vars and threads, so bound-fn is quite natural to express

Joshua Suskalo15:11:45

That's fair. In my particular usecase for clojurescript it'd really only be helping out with laziness. It's not a huge deal to not have it, just a bit unfortunate it's missing.