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

Support re-usable factory functions #247

Closed
janvanbouwel opened this issue Oct 19, 2022 · 8 comments
Closed

Support re-usable factory functions #247

janvanbouwel opened this issue Oct 19, 2022 · 8 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed question Further information is requested

Comments

@janvanbouwel
Copy link

Summary

Passing is<> as an argument does not get transformed, and there is no alternative like createIs<>(). In case this is intended behaviour, the error message is unhelpful.

  • Version: "typescript-json": "^3.3.12" (also on dev version)

Code example

import { is } from "typescript-json";

const verifier = is<string>;
console.log(verifier("abc"));

function test(verifier: (x: unknown) => x is string) {
    console.log(verifier("abc"));
}

try {
    test(verifier);
} catch (error) { console.log(error); }

try {
    test(is<string>)
} catch (error) { console.log(error); }

try {
    // ugly fix, especially with more complex types
    test((x): x is string => is<string>(x))
} catch (error) { console.log(error); }
  • Expected behavior:
true
true
true
true
  • Actual behavior:
true
Error: Error on TSON.is(): no transform has been configured. Configure the "tsconfig.json" file following the [README.md#setup](https://github.com/samchon/typescript-json#setup) at [...]
Error: [...]
true
@samchon samchon added good first issue Good for newcomers question Further information is requested labels Oct 19, 2022
@samchon samchon self-assigned this Oct 19, 2022
@samchon
Copy link
Owner

samchon commented Oct 19, 2022

It is possible to converting such is<T> statement without function call. Just by activating below code.

However, I head that such statement is sometimes dangerous because even mistaken code could be transpiled. Therefore, I need to get hear many users' ideas about this issue. Before listening and accepting or rejecting this issue, I recommend you to define function variable like below instead.

For reference, below hoisting function does not reduce performance when comparing with your suggestion.

import TSON from "typescript-json";
const verifier = (input: string) => TSON.is<string>(input);

@samchon samchon added the help wanted Extra attention is needed label Oct 19, 2022
@janvanbouwel
Copy link
Author

Thanks for the quick response! Although it works for now, my main issue with that solution is not performance but ugliness of having to write the type twice, making the code less readable and more annoying to change. I can't comment on the potential dangers, as I do not have experience with implementing transformations.

Solutions could be:

  • Reintroducing createIs<>()
  • Enabling that transformation with an extra transformation property, like functional and numeric (but disabled by default)

@samchon
Copy link
Owner

samchon commented Oct 19, 2022

I couldn't understand why typescript-is is providing such vulnerable functions like createIs() and createAssert().

However, listening your suggestion, I understood that why those functions are provided.

How do you think about in this case? Whether introducing making hoisting function would be better or creating those new functions like createIs() or createAssertType() would be better?

@janvanbouwel
Copy link
Author

I don't know what makes createIs() vulnerable. To me, using is<Type> directly feels more intuitive than having to use createIs<Type>() but if you want to avoid the former createIs() would work. (In this case the error message above could include "Error on TSON.is(), did you mean createIs()".)

Having to wrap is in an anonymous function each time (is that what you mean by hoisting?) is cumbersome and something I'd like to avoid.

@samchon
Copy link
Owner

samchon commented Oct 19, 2022

Okay, I will consider better solution and it would be published at this Sunday, maybe.

Thanks for suggestion.

@samchon samchon added the enhancement New feature or request label Oct 19, 2022
samchon added a commit that referenced this issue Oct 23, 2022
@samchon
Copy link
Owner

samchon commented Oct 23, 2022

I agree with you that such statement is right const checker = is<T>; checker(input).

However, there're some people who've migrated from typescript-is to typescript-json and the new functions should be same with the typescript-is for them. Therefore, below functions are newly added. Now, only writing README content is left, but if you wanna use it early, then install next tag:

npm install --save typescript-json@next
  • TSON.createAssertType()
  • TSON.createIs()
  • TSON.createValidate()
  • TSON.createAssertEquals()
  • TSON.createEquals()
  • TSON.createVallidateEquals()
  • TSON.createStringify()

@samchon samchon changed the title is<> function as argument Support re-usable factory functions Oct 23, 2022
@samchon
Copy link
Owner

samchon commented Oct 23, 2022

@janvanbouwel Now, update to v3.3.13, then you can use those above functions.

samchon added a commit that referenced this issue Oct 23, 2022
Develop #247 - reusable factory functions
@janvanbouwel
Copy link
Author

Works like a charm, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants