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

Wrong typescript definition for serializeError #79

Closed
ghost opened this issue May 31, 2022 · 4 comments
Closed

Wrong typescript definition for serializeError #79

ghost opened this issue May 31, 2022 · 4 comments

Comments

@ghost
Copy link

ghost commented May 31, 2022

If I am understanding the TypeScript correctly,

https://www.typescriptlang.org/docs/handbook/2/generics.html#working-with-generic-type-variables

https://github.com/sindresorhus/serialize-error/blob/main/index.js#L160

If the value input is function,

Should the type of return value must be also function as well?

If we still want to convert function value to string,

The type definition for input & return value should be any or unknown since they don't share the same type.

export function serializeError(error: any, options?: Options): any
@papb
Copy link

papb commented Jun 6, 2022

Should the type of return value must be also function as well?

No...

If we still want to convert function value to string,

According to this test, yes we do.

The type definition for input & return value should be any or unknown since they don't share the same type.

There's nothing wrong with input type and return type not sharing the same type.

@papb
Copy link

papb commented Jun 6, 2022

Also, in the future, prefer using a permalink when linking to code. So instead of

https://github.com/sindresorhus/serialize-error/blob/main/index.js#L160

You should use:

https://github.com/sindresorhus/serialize-error/blob/2a77169cae632802b75c9962b8ae9a482c978ae2/index.js#L160

To get this link:

image

@ghost
Copy link
Author

ghost commented Jun 6, 2022

There's nothing wrong with input type and return type not sharing the same type.

@papb What I wanted to say is that the current type definition is wrong

https://github.com/sindresorhus/serialize-error/blob/main/index.d.ts#L108

Type definition is enforcing that the return value should have the same type of the input, while we convert function inputs to string

@fregante
Copy link
Contributor

fregante commented Jun 8, 2022

Type definition is enforcing that the return value should have the same type of the input

That is correct. serializeError returns the same value if it's not an object and not a function

serialize-error/index.js

Lines 141 to 166 in 2a77169

export function serializeError(value, options = {}) {
const {
maxDepth = Number.POSITIVE_INFINITY,
useToJSON = true,
} = options;
if (typeof value === 'object' && value !== null) {
return destroyCircular({
from: value,
seen: [],
forceEnumerable: true,
maxDepth,
depth: 0,
useToJSON,
serialize: true,
});
}
// People sometimes throw things besides Error objects…
if (typeof value === 'function') {
// `JSON.stringify()` discards functions. We do too, unless a function is thrown directly.
return `[Function: ${value.name ?? 'anonymous'}]`;
}
return value;
}

The logic is simply:

  • if it's an object, return an object
  • if it's a function, return a string
  • if it's anything else, return it as is

The return is never unknown. Even if it is, we know it's Input, so we return it as such.

@sindresorhus sindresorhus closed this as not planned Won't fix, can't repro, duplicate, stale Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants