Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truss aliases fail in CLJS #64

Closed
p-himik opened this issue Nov 18, 2022 · 9 comments
Closed

Truss aliases fail in CLJS #64

p-himik opened this issue Nov 18, 2022 · 9 comments
Assignees

Comments

@p-himik
Copy link

p-himik commented Nov 18, 2022

The commit 6d7d7af attempts to introduce some aliases in CLJS. However, those don't allow for the CLJS code to be built:

------ ERROR -------------------------------------------------------------------
 File: jar:file:/home/p-himik/.m2/repository/com/taoensso/encore/3.35.0/encore-3.35.0.jar!/taoensso/encore.cljc:472:3
--------------------------------------------------------------------------------
 469 | ;;;; Truss aliases (for back compatibility, convenience)
 470 | 
 471 | (do
 472 |   (defalias taoensso.truss/have)
---------^----------------------------------------------------------------------
null
Unable to resolve var: have in this context at line 472 taoensso/encore.cljc
--------------------------------------------------------------------------------
 473 |   (defalias taoensso.truss/have!)
 474 |   (defalias taoensso.truss/have?)
 475 |   (defalias taoensso.truss/have!?)
 476 |   (defalias taoensso.truss/get-data)
--------------------------------------------------------------------------------

I suspect the reason is that in compiled CLJS, macros don't exist. They exist only in self-hosted CLJS. So you can't reference taoensso.truss/have or any other macro like that.

@ptaoussanis ptaoussanis self-assigned this Nov 18, 2022
@ptaoussanis
Copy link
Member

ptaoussanis commented Nov 18, 2022

@p-himik Hi Eugene, thanks for pinging about this!

I've unfortunately been unable to reproduce the problem you're seeing.
Could you please confirm the following?

  • Have you tried running lein clean or equivalent?
  • Does this also happen with Encore v3.35.1?
  • What version of Clojure & ClojureScript are you running?
  • Exactly what command are you running that triggers this error?

Thanks!

@p-himik
Copy link
Author

p-himik commented Nov 19, 2022

Ah, I completely misread the code, sorry. I can't reproduce it with just CLJS itself as well, but it's reproducible with shadow-cljs: https://github.com/p-himik/ptaoussanis-encore-issues-64

I've asked Thomas about it.

@ptaoussanis
Copy link
Member

No worries at all, it's good to know if there's a potential issue.

I'm not so familiar with shadow-cljs, so let's see what Thomas says - otherwise I'll try debug using your example project. Will make sure we arrive at some solution 👍

Cheers! :-)

@thheller
Copy link

thheller commented Nov 19, 2022

@p-himik your assessment is correct. Macros do not exist in CLJS, and this is the problem here.

The difference here is that shadow-cljs ignores defmacro when compiling .cljc files in CLJS mode. cljs.main does not. As such it compiles the macro as a regular function in CLJS. It'll never be called, but the var will exist.

I consider this an error, as it will produce invalid code that cannot ever be actually used. And in this case will even give you a callable taoensso.encore/have function, which can also never be used in a meaningful way.

@ptaoussanis also as an aside the CLJS variant of defalias macro is less than ideal. var in CLJS emits a whole bunch of boilerplate and just deref'ing it directly will leave all that code "alive" in :advanced. Leading to a lot of "bloat" and actually unused code. I would suggest just (def ~sym ~src), which will eliminate all that code, and yield the same result in the runtime.

@ptaoussanis
Copy link
Member

@thheller Thanks for the insight Thomas, that's super helpful!

[...] cljs.main does not. As such it compiles the macro as a regular function in CLJS. It'll never be called, but the var will exist. I consider this an error, as it will produce invalid code that cannot ever be actually used. And in this case will even give you a callable taoensso.encore/have function, which can also never be used in a meaningful way.

This is really interesting. I'd never actually looked into this, and had just presumed that defmacros in .cljc files would be ignored when compiling in CLJS. That's a pretty fundamental misunderstanding I've had then. Do you have any idea why they'd be compiled as a function? Is there some utility in that?

[...] also as an aside the CLJS variant of defalias macro is less than ideal.

I appreciate the feedback on this! I've actually never been happy with the defalias behaviour in Cljs, since the intention/hope was to also duplicate metadata like docstrings, etc. - but I never managed to get that to work correctly, and didn't get around yet to investigating further.

Do you happen to have any idea off-hand if there'd be some way to do this successfully?

In the meantime, I'll definitely proceed with your suggested improvements - cheers!

ptaoussanis added a commit that referenced this issue Nov 20, 2022
…s implementation (@theller)

`defalias` has never been able to successfully duplicate metadata in ClojureScript (see #53),
and @theller helpfully pointed out that the current attempt may have been including bloat in
:advanced output.
@ptaoussanis
Copy link
Member

@p-himik Hi Eugene, I just pushed [com.taoensso/encore "3.37.1"] to Clojars which removes all macros from ClojureScript - this should hopefully resolve your issue?

Thanks again for pinging about this, and apologies for the trouble!

@p-himik
Copy link
Author

p-himik commented Nov 20, 2022

That seems to work just fine, thanks! And it was no trouble at all.

@p-himik p-himik closed this as completed Nov 20, 2022
@thheller
Copy link

Metadata on vars doesn't really exist in the CLJS/JS runtime. It is only kept in the cljs.analyzer metadata "map". So, technically you can copy it out of there and copy it to the new var.


(defmacro defalias
  "Defines an alias for qualified source symbol, preserving its metadata (clj only):
    (defalias my-map-alias clojure.core/map)
  Cannot alias Cljs macros.
  Changes to source are not automatically applied to alias."
  ;; TODO Any way to reliably preserve cljs metadata? See #53, commit 2a63a29, etc.

  ([src] `(defalias ~(symbol (name src)) ~src nil))
  ([sym src] `(defalias ~sym ~src nil))
  ([sym src attrs]
   (let [attrs (if (string? attrs) {:doc attrs} attrs) ; Back compatibility
         link? (:link? attrs) ; Currently undocumented
         attrs (dissoc attrs :link?)]

     (if (:ns &env)
       (let [var (cljs.analyzer/resolve-var &env src)]
         `(def ~(vary-meta sym merge (:meta var)) ~src))
       `(let [attrs# (conj (-alias-meta (var ~src)) ~attrs)]
          (alter-meta! (def ~sym @(var ~src)) conj attrs#)
          (when ~link? (-link-var (var ~sym) (var ~src)))
          (var ~sym))))))

Something like that should do the trick. Although you may wanna add conditional requires or something, so you don't introduce a hard dependency on CLJS when using CLJ.

Given the limited usefulness of metadata in CLJS in the first place I doubt this is all worth it?

@ptaoussanis
Copy link
Member

Thanks a lot for the example Thomas, that was very helpful! Cheers :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants