Add a robustier instanceof alternative to allow cross-package compatibility.

[Shaptic]

Note: Lots of people tagged for review mostly as an FYI as there may or may not be far-reaching implications to this change.

What

Introduce a better check for cross-compatible XdrCompositeType subtypes (Union, Struct, Enum, and Array) based on a Discord discussion from @earrietadev which will check the prototype chain for the constructors and properties we need:

export function isSerializableIsh(
    value: any, 
    subtype: InstanceType<typeof XdrCompositeType>
): value is XdrCompositeType;

(This isn't a proper TypeScript definition because js-xdr (a) doesn't use TS and (b) doesn't use interfaces, but imagine XdrCompositeType as being an interface and it makes more sense.)

We also make the error messages way better.

Details

Because the classes are generated, their prototype constructor name doesn't match the actual class name more often than not. However, they do still inject the "true" name into their prototype. The three types we're trying to match on all do this differently:

• structs will use structName, see struct.js
• unions will use unionName, see union.js
• enums will use enumName, see enum.js

It's not perfect, but it's better than the instanceof check we have now.

So we will check these fields as well as the existence of a read() and write() method to make a best-effort guess that the given value matches the kind we expect.

Why

Major versions of a package should probably be cross-compatible in the same dependency tree. For example, js-xdr@3.1.1 should function interchangeably alongside js-xdr@3.0.0 if the additive features are not relevant.

That is not the case today due to instanceof checks that become package-specific, while isSerializableIsh should rectify this.

Known Limitations

There are only performance implications for people who are introducing this mixing behavior (besides the new overhead of a function call which I would consider trivial), so there are no performance regression risks.

[BlaineHeffron]

The new check also fails to resolve stellar/stellar-cli#1285

[willemneal]

One small nit, but I don't think it should prevent merging. Always fun digging into the guts of JS.

[willemneal]

I'm assuming that this is the Class, but personally I find it less confusing if you use the Class directly to remove any ambiguities. Plus the caller could have used bind to change this method's this. Not that we need to worry, but it's possible.

Suggested change

	if (!this.isValid(value)) {
	if (!Enum.isValid(value)) {

[Shaptic]

Unfortunately, we have to use this here for an oddly-specific reason: we need to make sure it deals with child classes and any modifications that are made after creation, e.g. like here. Just passing the class name doesn't cover this case.

[willemneal]

But shouldn't the classed be sealed? Why do we need to consider the case of subclasses?

[Shaptic]

You can see in that linked line that a Union itself is never actually created, it's always a locally-generated subclass of ChildUnion that has a bunch of properties attached to it (most importantly unionName) that gets returned. So we'd need to be able to [the specific ChildUnion class].isValid if we wanted an accurate evaluation of "this"ness.

[willemneal]

I've always had issues with this in JS and in this case being this as the Class or (constructor?) is a bit odd to me. But if this is the standard JS way, pay my reservations to heed.

[piyalbasu]

This looks solid to me. I'll echo @willemneal 's thought about using the Class rather than this. Probably not super critical, but you may as well cover all your bases

[Shaptic]

I've tried testing this with downstream systems and landed upon the wonderfully helpful "[object Object]'s struct name is SorobanAuthorizationEntry not SorobanAuthorizationEntry: " so clearly the extra check is still failing in key moments. I'm still in the process of debugging it and will merge this once I can pull that off.

[Shaptic]

Yooooo I ran this through a project that uses vite all the way down through the SDK and it works!! 🥳 🥳