Having both Ok and Err use the val variable opens the door to bad type checking.

[francescortiz]

If the ok and err type are the same, ts-results doesn't enforce type checking:

    const userIdOkTest: Result<string, string> = Ok("00287b3a-12fb-455c-b3d9-e662153c91b6");
    const userIdErrTest: Result<string, string> = Err("User not found");

    await doSomethingWithUser(userIdErrTest.val); // runtime error because the compiler didn't complain.
    
    // More examples that are not checked for ok or err
    if (userIdOkTest.val === "test") {
        console.log("ok or err not checked but it compiles.")
    } else if (userIdErrTest.val === "test") {
        console.log("ok or err not checked but it compiles.")
    }

This could be fixed if Ok and Err didn't use the same val attribute of the result instance.

[pXicoyFlexiDao]

any thoughts on this?

[jstasiak]

@francescortiz how would you change the API to resolve this problem?

[francescortiz]

I have created a PR showing the how it could be achieved: #70

[francescortiz]

I am thinking that it would be better to instead of having:

// OkImpl<T> fields
    readonly ok!: true;
    readonly err!: false;
    readonly okVal!: T;

// ErrImpl<E> fields
    readonly ok!: false;
    readonly err!: true;
    readonly errVal!: E;

Maybe it would be better to have:

// OkImpl<T> fields
    readonly ok!: true;
    readonly val!: T;

// ErrImpl<E> fields
    readonly ok!: false;
    readonly err!: E;

But err would have a completely different meaning this is case, which could make migrating to the new signature harder for existing projects in cases where booleans were used as error value.

[jstasiak]

FWIW we made a change addressing this issue in ts-results-es 4.0.0 (Some and Ok have value, Err has error).

[francescortiz]

FWIW we made a change addressing this issue in ts-results-es 4.0.0 (Some and Ok have value, Err has error).

Awesome, thank you!

[ctsstc]

Glad I found your fork @jstasiak that definitely helps with some issues I've had. Although many projects unfortunately have many struggles with utilizing 3rd party ESM currently 😢 (not sure if yours also handles the CJS export; I think I saw a comment or issue here or there about that).

Edit in case it helps with anyone else coming from Next JS, was getting the following error and your ES fork seems to work when this one did not.

Import trace for requested module:
./node_modules/ts-results/index.js
./src/services/queries/]profile.queries.ts
./src/app/social/[userHandle]/page.tsx

./node_modules/ts-results/option.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

Import trace for requested module:
./node_modules/ts-results/option.js
./node_modules/ts-results/index.js
./src/services/queries/profile.queries.ts
./src/app/social/[userHandle]/page.tsx

./node_modules/ts-results/result.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

[jstasiak]

@ctsstc ts-results-es can be used in all code – CJS and ESM