[RFC] enhancement(extension,repository): Add hidden __ident property to mocks and functions to differentiate them in conditional branching

[martinjlowm]

Differentiating mocked non-primitive function inputs is near impossible, especially if they are mocked through an IIFE-function. This PR adds a __factory __ident property on non-primitive mocks using Proxy, so they can be differentiated when used in conditional typing.

I'm open for ideas if this can be done any smarter. :) - It's similar to the Spy-identity property here: https://github.com/Typescript-TDD/ts-auto-mock/blob/master/test/frameworkContext/functions.test.ts

[uittorio]

Hi, Do you mind explaining a bit more in details why this work is required? I am sure there is a reason for conditional typing, but I would like to explore a bit more if there are any other approaches.

[martinjlowm]

Just throwing my response on Slack here for reference:

For mocks through interfaces, we cannot distinguish them at runtime they'll just be represented as anonymous objects, but for overloads and conditional typing we need some way to point out what type they are, so we can branch accordingly.
Primitive types are easy, there we can use typeof. At first I thought we could use instanceof, but again, the mocks are just anonymous objects and not tied to any constructor so that is not an option.

[Pmyl]

Could we reuse the mock marker if we want to go in this direction?

[uittorio]

Thats a good point @Pmyl ! We could use that.
However, I think we should not include mocks in the tyoe check for overloads because then we may have to check also for literal objects, not mocked interfaces, etc etc

I would first use this functionality only for pimitives types under a feature flag and then we need a define a solution that can work for other data types.

[martinjlowm]

I'm not sure I understand what you want to reuse the marker for. Do you mean storing the identity as the marker instead of the boolean value as it is right now? In that case, I suppose, we can even remove the emitted property definition and move that entirely to the proxy.
i.e.:

// eslint-disable-next-line
const marker = Symbol('__marker');

// eslint-disable-next-line
type Function<K> = (...args: any[]) => K;
type IdentityFlavored<K> = K & { [marker]?: string };

export function applyIdentityProperty<K extends object, T extends Function<K>>(target: T, identity: string): T {
  return new Proxy(
    target,
    {
      apply(func: T, _this: unknown, args: Parameters<T>): IdentityFlavored<K> | undefined {
        const t: IdentityFlavored<K> = func(...args);

        if (typeof t === 'undefined') {
          return;
        }

        if (!(t instanceof Object)) {
          return t;
        }

        if (typeof t[marker] !== 'undefined') {
          return t;
        }

        Object.defineProperty(t, marker, {
          enumerable: false,
          writable: false,
          value: identity,
        });

        return t;
      },
    },
  );
}

Right now, the marker instance is coupled with the extension module. If we are to wrap this at the registerFactory-level, we should externalize it somehow so both entries use the same instance.

[uittorio]

Hi all :) i think we got different options here. Im sure we can find the best approach to identify when something its mock or not.

We are definitely aware that this branch will not support all the scenario so in my opinion we should choose what to do next

1. find a solution that will support more scenarios (Interfaces, object literals, etc)

2. only support primitives and ship it :p

3. stop working on this functionality until we have more solutions. Martin will hate us :)

In my opinion I would investigate more if its fisible to support all yhe possible types, then we can incrementally deliver one piece at the time. My worry about developing this as it is is that when we will discover how to do literals, interfaces this code will be obsolete.

[uittorio]

Sorry Martin about this. @Pmyl and I are happy to write down all the scenario and maybe help out to find the technical solution. We can open a document in the wiki where we try to identify all the implication of overload

[martinjlowm]

Hehe, I'm not gonna hate You :p

A document sounds like a good idea.

To fully support literals, we would have to adjust the emitted descriptor to proxy those as well.

I tried to patch the proposed example above (martinjlowm@908af9c) and it results in the uncovered case of IIFE-emitted mocks (createMock<{ a: number }>() -> (function () { return ... })()) and for good reason would not work, because it never gets proxied.

I think the right way to go about this is to proxy everything through a factory and hash inputs based on type signature, so interfaces, classes, literals (that are not primitive) and even functions (anything that extends Object, really) can share the same factory - less code and smaller footprint 🎉 , the only problem is varying property names. I've been thinking about this for a couple of days, and I have yet to come up with a situation where it would be a problem. I suppose I can write a proof of concept.

The only other way I see to work around this identifier is to implement all mocks as classes, so the prototype can be referenced. It's basically what the proxy does, without emitting a lot of extra code.

[uittorio]

Hi, yes, we should create a document where we explain the concern about this.

This is the examples I'm not sure how you will identify correctly.

function test(a: string, InterfaceA): boolean;
function test(a: number, InterfaceB): string;

Let's say your are mocking test. We will have to start creating mocks of anything that its not primitive in functions calls. to find out later on if you are using test with InterfaceA or InterfaceB.

[martinjlowm]

I've given this some more thought and there are (as I see it) two directions we can go.

1. The opposite direction of this PR (that I originally intended), preserving the behavior as it currently is and use @Pmyl's idea to reuse the marker as the identity, leaving it to the emitted descriptor to inform the environment about its identity. I.e. emitted as such:

var m = {};
Object.defineProperty(m, extension.marker.instance.get(), { value: <identity> }); // previously: !0
return m;

Pros:

• Functionality remains as is and won't break anything. The On (extension handler) implementation currently only checks for a truthy value.

Cons:

• Potentially a lot of emitted code. This grows linear with the amount of interfaces that are to be mocked.

2. Move the marker to the proposed proxy and wrap everything, even literals passed directly to createMock and pass them through some sort of middleware that applies the proxy.

Pros:

• The property definitions are no longer emitted, but replaced with a middleware, some disguised as getMethod/getFactory. It may seem more magical, but the runtime is responsible of keeping track of all objects.

Cons:

• It completely changes how mocks are handled.
• There's a lot of refactoring/rewrite needed to make this fully functional.

I'm starting to lean towards option 1 because it is fully backwards compatible and requires the least rewrite. That is assuming, the marker isn't intended for something else down the road. Option 2 has a lot of unknowns, but it really comes down to what the end goal for this project is... having a complex runtime vs. a complex transformer that emits (possibly) redundant code (as I see it as I'm writing this). It's not to say it's impossible to go down a different road later on. As for conditional branching, the following two PRs are really just depending on the decision that is made here.

I would love your input, seeing as You brought the project to where it is today. :)

edit: One thing I forgot, is how to handle option 1 with methods, they currently don't supply a marker. Perhaps we can merge the two somehow. The only issue there is that methods aren't to be instantiated like factories are. Or, perhaps we can bind the function, so we can reference this within the function and apply the marker that way.

edit 2: Binding this won't work because it won't be accessible before the function is called, but something along the lines of the following will:

ɵExtension.Provider.instance.getMethod(
  "propertyName",
  (function () {
    var m = (function () {
      return void 0;
    });
    Object.defineProperty(m, ɵExtension.ɵMarker.instance.get(), { value: "propertyName/maybeSignature" });
    return m;
  })()
)

With new functions being created at the caller's perspective instead of here:

ts-auto-mock/src/extension/method/provider/functionMethod.ts

Line 4 in c40a61b

return (): any => value();

. The marker info might get lost if a method is provided with provideMethodWithDeferredValue and ends up returning something else than the specified function, e.g. a spy. I'm not sure if that will be a problem.

[martinjlowm]

Hi, yes, we should create a document where we explain the concern about this.

This is the examples I'm not sure how you will identify correctly.

function test(a: string, InterfaceA): boolean;
function test(a: number, InterfaceB): string;

Let's say your are mocking test. We will have to start creating mocks of anything that is not primitive in functions calls. to find out later on if you are using test with InterfaceA or InterfaceB.

An interesting note about this is that if interfaces (Interface)A and (Interface)B are identical, the type checker selects the first overload signature. Similarly, if you pass in a literal that is assignable to both interfaces, the first signature is also selected.

I think maybe that implies that anything you pass in which is assignable to InterfaceA, may actually reuse the factory of InterfaceA (that includes InterfaceB in this case) and we would avoid redundant code. Thus, leading to the following behavior.

declare function someFunction(a: A): string;
declare function someFunction(a: B): number;

interface A {
  a: number;
  b: string;
}
interface B {
  a: number;
  b: string;
}

const a: A = createMock<A>();
const b: B = createMock<B>();

const func: typeof someFunction = createMock<typeof someFunction>();
func(a); // Valid - someFunction(a: A): string;
func(b); // Valid - someFunction(a: A): string;
func({ a: 0, b: '' }); // Valid (from the type checker) - someFunction(a: A): string;

In addition to this, to limit the API of this framework, we can decide to only support mocked types, ones that have a marker, however, that would have to be a runtime check. I.e.:

const c: { a: 0; b: '' } = createMock<{ a: 0, b: '' }>();
func(c); // Valid - someFunction(a: A): string;

with the mocked implementation of func:

function (__0) {
  var identity = ɵRepository.ɵRepository.instance.getIdentity(__0); // If primitive, return null, else extract: __0[marker] and if it doesn't exist, throw a runtime error
  if (identity === '@A_1') {
    ...
  } else {
    ...
  }
}

Now, reusability, how do we accomplish this? We would need a way to check similarity of types and the proposal here: microsoft/TypeScript#9879 seems helpful - however, that is not implemented/exposed as of this writing, so we need some other way to do it. The first thing that comes to mind is to serialize the signatures and store them in a cache so they can be reused.

Right now, the transformer decides what to register for a factory based on declarations through getDeclarationKeyMap as WeakMap<ts.Declaration, string>. If we were to serialize signatures we could change getDeclarationKeyMap to use a cache that maps Map<string (serialized signature), string> instead.

Now the serialization is not a simple thing to implement and it quickly becomes complex. Here are some practical examples:

interface A {
  a: string;
} -> {a:string}

interface B {
  a: string;
  b: number;
} -> {a:string;b:number}

interface BReverse {
  b: number;
  a: string;
} -> {a:string;b:number} // Sorted by key

interface C {
  a: A;
} -> {a:@A_1} // This is the unique name from A, mocked.

interface D<T> {
  a: T;
} -> {a:T}

interface E {
  a(): void;
} -> {a:()->void}

class F {
  a(): F;
} -> {a:()->this}

declare function func(a: number): string; -> (number)->string

declare function funcOverload(a: number): string;
declare function funcOverload(a: string): number; -> [(number)->string;(string)->number]

I have a PoC right now for this that:

• Deprecates getMethod in favor of register-/getFactory and I believe fix(transformer): Ensure mocked interfaces don't extend themselves infinitely if passed as generic argument #312 will be fixed easily with this change.
• Deprecates provideMethodWithDeferredValue in favor of registerWrapper(method: MethodWithDeferredValue) in the repository. (backwards compatible)
• Moves the marker from the emitted factories into the proxy proposed in this PR and everything marker related is handled by the runtime. Adds getIdentity in the repository (not yet used, but will be for the overload/conditional typing).
• All tests pass without the serialization part implemented apart from some related to the frameworkContext tests (because the name (for the spies) from getMethod is now generated differently by getDeclarationKeyMap)

Right now, I am working on a first iteration of the serialization. What do you guys think? I hope I didn't miss anything crucial that this won't cover.

[uittorio]

Hi @martinjlowm, thanks for all these updates. I thought about it and I think we should find a better place to write down the proposal you are suggesting.

I'm using Notion as a platform to document, comments proposal. If you share with me your email I'm happy to send you the link and we can start typing the proposal. I've already filled some of it with requirements @Pmyl I'll send you also the link.