Fork me on GitHub
#cljs-dev
<
2022-03-02
>
juhoteperi10:03:48

Hey I noticed instance? calls checking for a record instances where record class is referred using qualified name broke between 1.10.339 and 1.10.439. (Or I guess referring to the type with qualified name broke, not the instance? check itself.) Code like (instance? schema.core.NamedSchema schema) where the namespace has require [schema.core :as s]. Should this work or is this just a expected difference with Clojure and I should deal with this with reader conditionals?

dnolen18:03:38

@juhoteperi schema.core/NameSchema doesn't work?

dnolen18:03:50

the dotted thing might be the issue

juhoteperi18:03:51

s/NamedSchema and schema.core/NamedSchema work, but they don't work on Clojure JVM side. This was on cljc namespace.

dnolen18:03:30

@juhoteperi as far as I can tell this was always by design

dnolen18:03:01

the foo.bar.baz stuff was never fully baked far as I could tell and there are still edgecases

dnolen18:03:47

if I recall forcing the issue foo.bar/baz actually fixed quite a few other ugly things, so this other convenience probably was the tradeoff

dnolen18:03:39

the important thing to realize is that foo.bar.baz is terrible w/o reified namesapces

dnolen18:03:43

no idea what that thing is

dnolen18:03:47

thus all the problems - not worth it

juhoteperi18:03:54

Okay. I was surprised with one older library where the tests started failing after upgrading from 1.10.339. Doesn't seem like this is a common problem anyway as this has worked like this for over 3 years.

dnolen18:03:19

it was a common problem - there were so many other bugs filed

juhoteperi18:03:42

Btw. the error is runtime exception. After looking at the JS output, and noticing this works in some other cases, I see the problem is local bindings with the same name as the namespace the dotted form uses:

;; works
(defn- single-sequence-element? [x]
  (instance? schema.core.One x))

;; broken
(defn- single-sequence-element? [schema]
  (instance? schema.core.One schema))

dnolen18:03:57

@juhoteperi this is what I'm talking about it was always broken

dnolen18:03:07

however it was before was broken in a different way

dnolen18:03:56

the important thing to understand is that what you wrote about as "broken"

dnolen18:03:13

could in fact be perfectly fine - because we have absolutely no clue what is going on

dnolen18:03:30

by forcing / all ambiguity about intention disappears

dnolen18:03:53

note previously what people were reporting was that had a local

dnolen18:03:56

if I remember - it's been a while - was I think unexpected shadowing for perfectly legitimate dotted access on a local

dnolen18:03:26

@juhoteperi I'm curious what is emitted w/ the / - there's actually another shadowing bug that was reported recently which seemed quite odd - probably not related but I'm curiuos

juhoteperi18:03:56

With s/One:

schema_tools.core.single_sequence_element_QMARK_ = (function schema_tools$core$single_sequence_element_QMARK_(schema__$1){
return (schema__$1 instanceof schema.core.One);
});
With schema.core.One the dotted name is changed to schema__$1.core.One also.

dnolen18:03:34

right that is absolutely correct

dnolen18:03:45

because dotted access on locals was meant to work

dnolen18:03:44

so that is probably why it was done

juhoteperi18:03:13

Yeah. This makes sense. I wonder if e.g. the Differences to Clojure doc page should mention something about possible differences to Clojure? Clojure The Reader page mentions: > '.' has special meaning - it can be used one or more times in the middle of a symbol to designate a fully-qualified class name, e.g. java.util.BitSet, or in namespace names. Symbols beginning or ending with '.' are reserved by Clojure. Symbols containing / or . are said to be 'qualified'.

dnolen18:03:41

we could but it's all very strange and interop / host stuff

dnolen18:03:52

JS doesn't have classes / packages in the Java sense

dnolen18:03:45

schema.core.One is really a JVM thing to get at the backing class of the deftype

dnolen18:03:48

or whatever

dnolen18:03:53

I guess we could call out that there are no classes/packages in the Java sense and such patterns should not be considered portable

dnolen18:03:30

@juhoteperi a just had a walk - one thought we could special case instance? ?

dnolen18:03:12

probably should be done for implements? and satisfies? too?

dnolen18:03:27

basically these are static anyway - if given a fully dotted symbol - could automatically namespace

juhoteperi19:03:27

Perhaps. Special cases don't sound nice, but it is hard to see a case where instance? (& others) call would use dotted symbol to access property in local binding, so it makes sense.

dnolen19:03:27

@juhoteperi yeah, but this probably a wide surface of commonality that would require conditional reading otherwise

juhoteperi19:03:46

(fn [schema] (instance? (.. schema -foo -bar) schema)) would still work to access properties? And this is the recommended way to access props anyway, over dotted name,

dnolen19:03:15

@juhoteperi still a local clash

juhoteperi19:03:38

I mean in that example it definitely should use the local over schema namespace, and it currently does

dnolen19:03:38

to be clear, since the very beginning foo.bar.baz was supported on locals