Singletons are not singletons in Typescript

[andreaTP]

In Apicurio Registry we are attempting to use the client SDK generated for Typescript ( link ).

More specifically, we are following the consolidated pattern of:

• generating the SDK standalone (we will publish it eventually)
• use it as a package from the UI application

Although we started running into issues:

After much research and debugging we found out that ParseNodeFactoryRegistry.defaultInstance doesn't always refer to the same object, and, depending on: "from which module are you invoking it" you get a different instance of it ( there are multiple resources in SO that refer to this e.g. ).

Workaround

Running kiota generate --serializer none --deserializer none and registering manually the serializers and deserializers in the end module, i.e.:

const pnfr = new ParseNodeFactoryRegistry();
const json = new JsonParseNodeFactory();
...
    
pnfr.contentTypeAssociatedFactories.set(json.getValidContentType(), json);
...

new FetchRequestAdapter(new AnonymousAuthenticationProvider(), pnfr, ...);

Solution?

This bug is hard to discover and track down, and it easily compromises the "getting started" experience. There are a few options on how to solve it, but I'm happy to listen to a proposal from someone more expert than me in TS.

[andreaTP]

cc. @EricWittmann

[baywet]

Thanks for bringing this up. It's most likely caused because the bundled application actually has two copies of the abstractions, and yes that's a pain. Hard to investigate for people new to kiota and/or to JavaScript.

As part of making the clients truly portable, we're planning to build bundle packages which would do two things:

• refer to abstractions + all the default implementations (except for auth maybe)
• register the serialization providers (through a derived request adapter, not through the singleton registry anymore) This should solve this issue. We'll still have a singleton for the backing store factory, which could pause similar issues, but we don't have plans for that yet.

[andreaTP]

It's most likely caused because the bundled application actually has two copies of the abstractions

Haven't realized! Why?

register the serialization providers through a derived request adapter

Do you mean attaching the defaultInstance to the adapter? In this case, everything will flow from the instantiation of the adapter. Would this work even in case instantiation happens in a factory distributed along the SDK? I mean in a separate module.

[baywet]

Haven't realized! Why?

If you run npm list @microsoft/kiota-abstractions (or equivalent with your package manager) it's likely that it'll list two versions that haven't been deduped.

Do you mean attaching the defaultInstance to the adapter?

No I meant basically moving the code that's in the client now to register the providers to this new adapter's constructor. And change it a little bit so it'd:

1. accept providers passed to the constructor by the developer, and use those if any.
2. if none, use what's in the default instance if any.
3. if none, use the providers to register, and set that for the default instance at the same occasion.

Would this work even in case instantiation happens in a factory distributed along the SDK? I mean in a separate module.

I'm not sure I follow the question?

[baywet]

This is partially addressed by #1298

We could at this point decide to remove the "singletons" and special case TypeScript in the generator to NEVER have the client register serialization provider (workspace experience or not).

Thoughts @andreaTP ?

[andreaTP]

Sorry for the delay @baywet , the linked PR is still relying on defaultInstance, I have not tested, but I'm not sure how it addresses this issue.

NEVER have the client register serialization provider

This, or you find a way to have everything referencing fields of a RequestAdapter (and use proper DI everywhere).

[rkodev]

@baywet just making sure we are on the same page

No I meant basically moving the code that's in the client now to register the providers to this new adapter's constructor. And change it a little bit so it'd:

1. accept providers passed to the constructor by the developer, and use those if any.

We already have this implemented on the Fetch Adapter, and should be considered done

2. if none, use what's in the default instance if any.

same as point 1 above

3. if none, use the providers to register, and set that for the default instance at the same occasion.

How different will this be from the registration function we already have

From what I understand we still retain the defaultInstance in the registry which is what we want to eliminate

[baywet]

I'll try to explain what I was thinking of another way:

1. remove default instances in the registries.
2. move any static methods that depend on it as instance methods.
3. update the generation code to rely on the instance that's in the request adapter instead.

Doing that, we shouldn't have anything static anymore.

Does that make more sense?

[andreaTP]

I like it, it's possibly an improvement for all implementations 🙂

[baywet]

would be a breaking change for other languages at this point, but once this one is done, we could at least create the issues on the other repos.

[andreaTP]

would be a breaking change for other languages at this point

I think that it can be rolled out as a non-breaking change:

1. keep the default instances, only deprecate the singletons
2. implement instance methods to fallback on the deprecated singletons
3. update the codegen to rely on the instance methods

in a future release/cleanup the default singletons can be removed.