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

extending JS builtins #242

Closed
lilactown opened this issue Jul 24, 2022 · 4 comments
Closed

extending JS builtins #242

lilactown opened this issue Jul 24, 2022 · 4 comments

Comments

@lilactown
Copy link
Contributor

lilactown commented Jul 24, 2022

version

v0.6.129

platform

macOS 12.4 + Nodejs v18.3.0

problem

ClojureScript allows extending protocols to built in types like strings, numbers and general objects without mutating their prototypes by using special "type symbols." This is documented in the docs for extend-type. See this google groups discussion for more discussion.

nbb might not need to worry about mutating the prototype - I haven't dug into how nbb & sci implement protocol dispatch - but I think we ought to support CLJS libs that follow this convention, such as my own cascade which extends a protocol to the object symbol.

repro

There's slightly different behavior depending on what "type symbol" one attempts to extend.

Most of them print an error message at the REPL.

Welcome to nbb v0.6.129!
user=> (defprotocol IFoo)
IFoo
user=> (extend-type object IFoo)
"#error {:message \"Could not resolve symbol: object\", :data {:type :sci/error, :line nil, :column nil, :file nil, :phase \"analysis\"}}"
user=> (extend-type string IFoo)
"#error {:message \"Could not resolve symbol: string\", :data {:type :sci/error, :line nil, :column nil, :file nil, :phase \"analysis\"}}"
user=> (extend-type number IFoo)
"#error {:message \"Could not resolve symbol: number\", :data {:type :sci/error, :line nil, :column nil, :file nil, :phase \"analysis\"}}"
user=> (extend-type function IFoo)
"#error {:message \"Could not resolve symbol: function\", :data {:type :sci/error, :line nil, :column nil, :file nil, :phase \"analysis\"}}"

However, some of them seem to succeed but don't have the desired behavior.

nil stands out as seeming to work, but doesn't seem to actually extend the protocol to nil values.

user=> (extend-type nil IFoo)
{:methods #{}, :name user/IFoo, :ns #object[sci.lang.Namespace], :sigs {}, :var #'user/IFoo, :satisfies #{}}
user=> (satisfies? IFoo nil)
false

array and boolean do something even weirder: they extend the protocol to the core array and boolean functions, which doesn't have the desired effect.

user=> (extend-type array IFoo)
{:methods #{}, :name user/IFoo, :ns #object[sci.lang.Namespace], :sigs {}, :var #'user/IFoo, :satisfies #{function cljs$core$array(var_args){
var a = (new Array(arguments.length));
var i = (0);
while(true){
if((i < a.length)){
(a[i] = (arguments[i]));

var G__27645 = (i + (1));
i = G__27645;
continue;
} else {
return a;
}
break;
}
}}}
user=> (satisfies? IFoo #js [])
false

expected behavior

Extending a protocol in any of the ways above would not result in an error, and would ensure (satisfies? IFoo x) would return true for the value x of the associated built in created by either literal or constructor.

@borkdude
Copy link
Collaborator

Thanks. I think this might be due to how the words object etc are resolved in SCI. Can you also try with js/Object, js/String, etc?

@lilactown
Copy link
Contributor Author

Hmm, using js/String and js/Object do not seem to work either

Welcome to nbb v0.6.129!
user=> (defprotocol IFoo)
IFoo
user=> (satisfies? IFoo "")
false
user=> (extend-type js/String IFoo)
{:methods #{}, :name user/IFoo, :ns #object[sci.lang.Namespace], :sigs {}, :var #'user/IFoo, :satisfies #{function String() { [native code] }}}
user=> (satisfies? IFoo "")
false
user=> (extend-type js/Object IFoo)
{:methods #{}, :name user/IFoo, :ns #object[sci.lang.Namespace], :sigs {}, :var #'user/IFoo, :satisfies #{function String() { [native code] } function Object() { [native code] }}}
user=> (satisfies? IFoo "")
false
user=> (satisfies? IFoo #js {})
false

@borkdude
Copy link
Collaborator

This might be an issue with how satisfies? is implemented in SCI when you have no methods.

This seems to work:

Welcome to nbb v0.6.129!
user=> (defprotocol IFoo (foo [_]))
IFoo
user=> (extend-type js/String IFoo (foo [_] :string))
#object[Object [object Object]]
user=> (foo "foo")
:string
user=> (satisfies? IFoo "")
true

@lilactown
Copy link
Contributor Author

Reading the source of sci.impl.protocols lead me to another bug in how nbb handles extending to default:

Welcome to nbb v0.6.129!
user=> (defprotocol IFoo (foo [_]))
IFoo
user=> (extend-type default IFoo (foo [_] "bar"))
#object[cljs.core.MultiFn]
user=> (foo #js {})
"bar"
user=> (def o (js/Object.create nil)) ;; create an object whose prototype is not `Object`
#'user/o
user=> o
#object[Object]
user=> (foo o)
"#error {:message \"No implementation of method: :foo of protocol: #'user/IFoo found for: \", :data {:type :sci/error, :line 1, :column 1, :message \"No implementation of method: :foo of protocol: #'user/IFoo found for: \", :sci.impl/callstack #object[cljs.core.Volatile {:val ({:line 1, :column 1, :ns #object[sci.lang.Namespace], :file nil, :sci.impl/f-meta {:name foo, :ns #object[sci.lang.Namespace], :file nil}} {:line 1, :column 1, :ns #object[sci.lang.Namespace], :file nil, :sci.impl/f-meta {:name foo, :ns #object[sci.lang.Namespace], :file nil}})}], :file nil}, :cause #object[Error Error: No implementation of method: :foo of protocol: #'user/IFoo found for: ]}"

this is due to how SCI's extend-type maps default to js/Object instead of having it be a fall through in the type hierarchy.

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

No branches or pull requests

2 participants