@henryw374 this is doesn't affect most libs in CLJSJS, so this isn't needed - it's easy to miss the details here
it's also a potential caching issue (you might not hit, because you install your deps first), not a overall support problem
anyways I don't want to go over it again - just sitting on it for now until there's more feedback and better ideas
flags don't really help at all because it's not about configuration - only caching
oh I see. I was thinking that dropping cljsjs deps would become the preferrred thing for libs to do generally
yes so users don't inadvertently hit the caching issue
which can always be fixed by blowing away ~/.cljs
and your build
and no, dropping cljsjs deps is not preferred
no interest in breaking anything and dropping support for anything
anything that worked before will continue to work
the only time we've ever not followed that was because the maintenance is impractical and the affected users are too small
i.e. the less popular REPLs
cool. I still think the flag would be good, so non-shadow npm users don't have to track down and exclude cljsjs deps. the flag could work so you set it to ignore foreign-libs - so it's an opt-in
more flags means more confusion - so just not interested at the moment
fair enough. there are quite a few flags already
too many
but it's still way better than most JS tooling - let's keep it that way
:thumbsup:
“There’s a flag for that” ™️
https://github.com/clojure/clojurescript/blob/0eaa19f4326f02d4dc4e8660ad5f13329b73e3af/src/main/clojure/cljs/analyzer/api.cljc#L171-L178 this docstring seems misleading given that it doesn't use *compiler*
and actually rebinds *compiler*
I think it was lifted from https://github.com/clojure/clojurescript/blob/0eaa19f4326f02d4dc4e8660ad5f13329b73e3af/src/main/clojure/cljs/analyzer.cljc#L4721-L4725 which does have that behavior
@dnolen https://github.com/clojure/clojurescript/commit/45022fa177dc900b774c6736e04e698426bf55c8#diff-3f5db04e51ac4262a2042aa4d80010a2L137-R131 looks like this commit (and line number) changed the behavior of the function. This has broken cljdoc upstream.
docstring patch would be cool! 🙂
@dnolen just to clarify - does that mean it's intentional that it no longer uses cljs.env/*compiler*
(it used to)
Maybe cljdoc shouldn't be using that API if it's unstable. Is there an alternative?
it uses empty-state
from the analyzer api so that the dyn var can become a detail
I've tried to start hiding the dyn vars, you can see this in the various changes to the api
and the dogfooding as in this case
OK. No problem. In that case I'll merge the change to cljdoc which will explicitly pass the state in instead :)
If I don't forget, I'll put a docstring patch together (I'll check any other relevant fns from that commit too).