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

type should return something else than PersistentMap for defrecord? #492

Closed
borkdude opened this issue Jan 7, 2021 · 7 comments
Closed

Comments

@borkdude
Copy link
Collaborator

borkdude commented Jan 7, 2021

(defrecord Foo []) (type (Foo.))

In Clojure a Class is returned:

user=> (defrecord Foo [])
user.Foo
user=> (type (type (Foo.)))
java.lang.Class

In ClojureScript a function is returned:

cljs.user=> (defrecord Foo [])
cljs.user/Foo
cljs.user=> (type (type (Foo.)))
#object[Function]

I'm not sure what to return in sci, because it does not define classes like Clojure.

@borkdude borkdude changed the title type should return symbol of defrecord type should return something else than PersistentMap for defrecord? Jan 7, 2021
mk added a commit to mk/sci that referenced this issue Jan 8, 2021
@mk
Copy link
Collaborator

mk commented Jan 8, 2021

For our use case, it would be sufficient, if we had some way, of getting to the fully-qualified class name.

In Clojure this works:

(ns user)
(defrecord R [foo])
(-> "bar" ->R type .getCanonicalName) ;; => "user.R" 

In sci:

(ns user)
(defrecord R [foo])
(-> "bar" ->R meta :sci.impl/type str) ;; => "R"

@mk
Copy link
Collaborator

mk commented Jan 8, 2021

I've added a failing test in #493 but haven't been able to get it working yet.

@borkdude
Copy link
Collaborator Author

borkdude commented Jan 8, 2021

@mk Thanks. I'll have a look during the weekend, hopefully.

@borkdude
Copy link
Collaborator Author

borkdude commented Jan 8, 2021

@mk How would you tackle this for normal CLJS?

@borkdude
Copy link
Collaborator Author

borkdude commented Jan 9, 2021

@mk In branch record-type, commit a40d4a7, this works:

$ lein run -m sci.impl.main "(ns foo) (defrecord Foo []) (type (Foo.))"
foo/Foo

Note that type returns a symbol here and not a class, like in Clojure. Also it's namespaced, not dotted. Not sure if that matters?

@borkdude
Copy link
Collaborator Author

borkdude commented Jan 9, 2021

@mk I found in clara rules how they solve this for CLJS:

(ns foo)

(defrecord Foo [])

(derive Foo ::system-type)

(isa? (type (Foo.)) ::system-type)

I think that could be a useful way to not rely on the specifics of the return value of type but still get the information you need out of it. I wrote a test for this in sci now (in the branch).

borkdude added a commit that referenced this issue Jan 9, 2021
@borkdude
Copy link
Collaborator Author

borkdude commented Jan 9, 2021

@mk With the changes just merged to master it should now be possible to do what you want.

@borkdude borkdude closed this as completed Jan 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants