Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

try/tryit is incompatible with certain function overloads #60

Closed
shockch4rge opened this issue Sep 2, 2022 · 8 comments
Closed

try/tryit is incompatible with certain function overloads #60

shockch4rge opened this issue Sep 2, 2022 · 8 comments
Labels
help wanted Extra attention is needed typing-issue

Comments

@shockch4rge
Copy link

I have been trying to use try with fs.readdir from Node's fs/promises module. The typical function signature you would use is simply:

const fileNames = await fs.readdir(path) // <- returns an array of strings  

Unfortunately, fs.readdir has 3 other overloads:

function readdir(
        path: PathLike,
        options:
            | {
                  encoding: 'buffer';
                  withFileTypes?: false | undefined;
              }
            | 'buffer'
    ): Promise<Buffer[]>;

function readdir(
        path: PathLike,
        options?:
            | (ObjectEncodingOptions & {
                  withFileTypes?: false | undefined;
              })
            | BufferEncoding
            | null
    ): Promise<string[] | Buffer[]>;

function readdir(
        path: PathLike,
        options: ObjectEncodingOptions & {
            withFileTypes: true;
        }
    ): Promise<Dirent[]>;

Using try with fs.readdir results in the last overload being used, making the second function parameter options required (meaning I can't omit it at all) and returning Dirent[] in the promise.

// fileNames is now 'Dirent[]', not 'string[]'
const [err, fileNames] = await _.try(fs.readdir)(path, { 
    // withFileTypes is constrained to be 'true'
    withFileTypes: true,
 })

A possible workaround would be to place fs.readdir into a temp function like so:

const read = fs.readdir(path);
const [err, fileNames] = await _.try(read)(); // <- returns string[]

However, I find that the workaround loses its elegance in utility libraries like radash or lodash.

@sodiray
Copy link
Owner

sodiray commented Sep 2, 2022

This is a valid point. I struggle with it on occasion as well. If you, or anyone, knows how to make the typings more robust I'm all ears.

Your workaround is interesting! In cases where TS gives me trouble I just inline the function:

const [err, fileNames] = await _.try(() => fs.readdir(path))()

In other cases:

const [err, workspace] = _.try(async () => {
  const users = await api.users.list()
  const projects = await api.projects.list()
  return map(projects, users)
})()

@shockch4rge
Copy link
Author

Ah, I think the way you work around it is great. As for a pull request, I'm currently quite new to TypeScript, so I think I'll only come back in the future to make some changes haha.

I'll be closing this issue as I don't think there's a need to keep it open 👍

@shockch4rge
Copy link
Author

Actually, I'll leave it open for others to make changes to the types. Sorry about that!

@shockch4rge shockch4rge reopened this Sep 2, 2022
@sodiray sodiray added the help wanted Extra attention is needed label Sep 2, 2022
@sodiray
Copy link
Owner

sodiray commented Sep 2, 2022

If any Typescript wizards stumble onto this and know how to curry a function in TS and return a new function with the same types -- including the overloads, please do share.

@Arash1381-y
Copy link

If any Typescript wizards stumble onto this and know how to curry a function in TS and return a new function with the same types -- including the overloads, please do share.

hello there =)
I think I'm not the wizard you wish for because I have just started to learn typescript after seeing this issue. =")

I believe, there must not be a way to handle this issue. Based on my very little knowledge, in runtime Node will choose which function must be called based on the way that a function is called. in "tryit" function, the input func is not called but its argument is copied which I think makes the node choose which signature is appropriate for that input func. based on my testing for overloaded functions, the last definition is the chosen one.

similar scenario:
I have checked apply function for tslib which is written by mozila and even apply couldn't handle this behavior and if you try to use it, it says that readdir must have 2 args. (same problem)

So I think there is no solution for overloaded functions, really =). I will be happy to know your opinion about this.

Screenshot (287)

@sodiray
Copy link
Owner

sodiray commented Jan 6, 2023

Hey @Arash1381-y 👋🏻 I thought (mostly assumed) the same. It's awesome to see a concrete test with another lib that shows some proof.

It's a bit of a bummer it isn't possible but it is an easy workaround to embed the arrow function inside tryit.

@matteosacchetto
Copy link
Contributor

Ok, so I was looking into this and I think this has no straightforward solution.

According to this issue the problem with inference when used with overloads seems to be a design limitation. So I don't think we can have a simple way which solves this without some kind of workaround.

To address this issue with the try/tryit functions (and any other similar functions) the workaround I found are:

  • Wrap the function in a simple lambda
  • Wrap the function in a lambda where the parameter of the lambda are the ones you pass to the inner function
  • You explicitly set the TFunction generic parameter setting it to the overload you are interested in.

To demonstrate these three alternatives I will use the readdir function shown in the issue

import { readdir } from 'fs/promises';

Lambda wrapping

const try_readdir = tryit(() => readdir(path))
const res = await try_readdir(); // [Error, undefined] | [undefined, string[]]

Lambda wrapping with params

const try_readdir = tryit((path: string) => readdir(path))
const res = await try_readdir('./dir'); // [Error, undefined] | [undefined, string[]]

Explicit overload

const try_readdir = tryit<
  (
    path: PathLike,
    options?:
      | (ObjectEncodingOptions & {
          withFileTypes?: false | undefined;
        })
      | BufferEncoding
      | null
  ) => Promise<string[]>
>(readdir)

const res = await try_readdir('./dir'); // [Error, undefined] | [undefined, string[]]

Hope this helps

@sodiray
Copy link
Owner

sodiray commented Jun 27, 2023

I'm going to close this as an unfortunate limitation with a known workaround.

@sodiray sodiray closed this as completed Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed typing-issue
Projects
None yet
Development

No branches or pull requests

4 participants