neverthrow: Problem with combine() and Mocks

I just spent the last few days chasing my tail on this one. I was using several mocking libraries, notably ts-mockito and testdouble, and got the same behavior. If I created a mock object, wrapped it with okAsync(), and tossed that into combine(), I would get nothing in the return- the mock object was being dropped. Here’s an example test in jest that would fail:

import td from "testdouble";

interface ITestInterface {
    getName(): string;
    setName(name: string): void;
    getAsyncResult(): ResultAsync<ITestInterface, Error>;
}

describe("Debugging and basic info tests", () => {
test("combine works with TestDouble mocks of interfaces", async () => {
    // Arrange
    const mock = td.object<ITestInterface>();

    // Act
    const result = await combine([okAsync(mock)]);

    // Assert
    expect(result).toBeDefined();
    expect(result.isErr()).toBeFalsy();
    const unwrappedResult = result._unsafeUnwrap();
    expect(unwrappedResult.length).toBe(1);
    expect(unwrappedResult[0]).toBe(mock);
  });
});

If you run this, you’d find that the last two expect calls would fail, because combine would return an empty array! It would only do this with mocks, not with normal objects. So I dug into it, and I found a solution in the combine() code. You are using array.concat(nonArray) in the reduce() of combineResults(). I rolled my own combine and it works even with mocks. I added typing support for heterogenous lists as well, but it looks like you’re working on a slicker solution in another issue. Here’s my “fixed” combine:

export class ResultUtils {
    static combine<T, T2, T3, T4, E, E2, E3, E4>(asyncResultList: [ResultAsync<T, E>, ResultAsync<T2, E2>, ResultAsync<T3, E3>, ResultAsync<T4, E4>]): ResultAsync<[T, T2, T3, T4], E | E2 | E3 | E4>;
    static combine<T, T2, T3, E, E2, E3>(asyncResultList: [ResultAsync<T, E>, ResultAsync<T2, E2>, ResultAsync<T3, E3>]): ResultAsync<[T, T2, T3], E | E2 | E3>;
    static combine<T, T2, E, E2>(asyncResultList: [ResultAsync<T, E>, ResultAsync<T2, E2>]): ResultAsync<[T, T2], E | E2>;
    static combine<T, E>(asyncResultList: ResultAsync<T, E>[]): ResultAsync<T[],E> {
        return ResultAsync.fromPromise(Promise.all(asyncResultList), 
        (e) => {return e as E})
        .andThen(ResultUtils.combineResultList);
    }
    
    static combineResultList<T, E>(resultList: Result<T, E>[]): Result<T[], E> {
        return resultList.reduce((acc: Result<T[], E>, result) => {
            return acc.isOk()
                ? result.isErr()
                    ? err(result.error)
                    : acc.map((values) => {
                        values.push(result.value);
                        return values;
                    })
                : acc;
        }, ok([]));
    };
}

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 21 (16 by maintainers)

Commits related to this issue

fix(utils/combine): not appending all proxies due to Array.prototype.concat internals issue: #228 — committed to kieran-osgood/neverthrow by kieran-osgood 3 years ago

Most upvoted comments

@supermacro That’s great! How do you usually do this, I can give you my twitter to speak easier? https://twitter.com/kieranbosgood

kieran-osgood on Nov 19, 2021

Hey @kieran-osgood, sorry for the delay in responding.

I’ll be sinking my teeth into this issue and your response once again this week.

supermacro on Nov 15, 2021

TLDR

This will be a bit lengthy, the first snippet is about the undefined logging in the maps, but the final two snippets are the ones where changes were made that matter for the original issue. The rest was just exploratory stuff incase it helps prompt more thoughts if I’m missing something important

I just wanted to play around and I seem to have gotten a case that works for what was shown above but I hope someone might be able to help verify this.

Undefined logging of snippets provided above

So repo you posted above (not the repl.it) I think the issue with the lines you’ve mentioned logging undefined is because you’re console logging the result of string + result value (which in the case of const mock = td.object<ITestInterface>() will be an object) e.g. if you were to adjust your code to this:

 // inside of node_modules/neverthrow/dist/index.cjs.js
 var combineResultList = function (resultList) {
    console.log(resultList[0].value) // <---- value is _NOT_ undefined here

    var resultList2 = resultList.map((result) => {

     // Note using the comma, not the +
      console.log('> ', result.value) // <----- value is undefined here no longer!
      return result
    })
   
    // ... other stuff
};

Theres no undefined logs coming out, infact logging out the ‘mock’ variable will show just an output of { toString: [Function (anonymous)] }, going further and logging mock.getName does give [Function: testDouble] { toString: [Function (anonymous)] } - I’m not familiar on proxies but thats supposedly the way they’re handling intercepting these calls to these objects and replacing them with whatever mocks are provided, which was throwing me off as being related, see below

Something weird I did notice is that the mock value seems to change after the combine?

describe("Debugging and basic info tests", () => {
  it("combine works with TestDouble mocks of interfaces", async () => {
    // Arrange
    const mock = td.object<ITestInterface>()
    console.log('check mock 1', mock);

    // Act
    const result = await combine([
      okAsync(mock),
      okAsync({ name: 'giorgio' }),
      okAsync(123),
    ] as const)

    console.log('check mock 2: ', mock); // second mock now is different 

    // Assert
    expect(result).toBeDefined()
    expect(result.isErr()).toBeFalsy()
    const unwrappedResult = result._unsafeUnwrap()

    expect(unwrappedResult.length).toBe(2)
    expect(unwrappedResult[0]).toBe(mock)
  })
})

the “check mock 2” log now has a value of:

 console.log
    check mock 2:  {
      toString: [Function (anonymous)],
      length: [Function: testDouble] {
        toString: [Function (anonymous)],
        [Symbol(Symbol.toPrimitive)]: [Function: testDouble] { toString: [Function (anonymous)] }
      },
      [Symbol(Symbol.isConcatSpreadable)]: [Function: testDouble] { toString: [Function (anonymous)] }
    }

Possible Solution

(it gets the tests passing, but I don’t have full explanation as to why and am looking more)

As the author originally pointed out that its related to the concat method, im assuming its related back to the proxy implementation of this, but overall (and im not sure if theres any impacts to these changes so take a look and see if it prompts anything in anyone who knows this better than me, because its weird that this “worked”?) but it seems like it was trying to unfurl the objects as well (like it would with an array) so:

var appendValueToEndOfList = function (value) { return function (list) {
    // need to wrap `value` inside of an array in order to prevent
    // Array.prototype.concat from destructuring the contents of `value`
    // into `list`.
    //
    // Otherwise you will receive [ 'hi', 1, 2, 3 ]
    // when you actually expected a tuple containing [ 'hi', [ 1, 2, 3 ] ]
    if (Array.isArray(value) || typeof value === 'object' && value !== null) {
        return list.concat([value]);
    }

    return list.concat(value);
}; };

resulted in all of these assertions working

    expect(unwrappedResult.length).toBe(3)
    expect(unwrappedResult[0]).toBe(mock)
    expect(unwrappedResult[1]).toStrictEqual({name: 'giorgio'})
    expect(unwrappedResult[2]).toEqual(123)

kieran-osgood on Nov 15, 2021

bump

delitescere on Jul 13, 2021

Not a problem. Fortunately I have a workaround that’s acceptable; I just wanted to let you know and hopefully save others the same headaches!

HyperCharlie on Feb 1, 2021