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

$Call with arguments has confusing documentation #149

Closed
kumar303 opened this issue Aug 13, 2020 · 6 comments
Closed

$Call with arguments has confusing documentation #149

kumar303 opened this issue Aug 13, 2020 · 6 comments

Comments

@kumar303
Copy link
Contributor

kumar303 commented Aug 13, 2020

Description

The documentation for $Call is unclear:

// Common use-case
const add = (amount: number) => ({ type: 'ADD' as 'ADD', payload: amount });
type AddAction = $Call<typeof returnOfIncrement>; // { type: 'ADD'; payload: number }

What is returnOfIncrement and where does it come from?

I read #21 but I still don't understand. Thanks for your help.

Steps to Reproduce

  1. Try to port Flow to TS using $Call where $Call originally was given two arguments
  2. Read the docs to figure out how to convert this code to TS

Expected behavior

The docs should provide a more clear example of how to convert old Flow style usage of $Call with arguments.

Suggested solution(s)

Provide an example of working TypeScript code in the docs

Project Dependencies

  • Utility-Types Version: 3.10.0
  • TypeScript Version: 3.9.7
tsconfig.json
{
  "compilerOptions": {
    /* Visit https://aka.ms/tsconfig.json to read more about this file */

    /* Basic Options */
    // "incremental": true,                   /* Enable incremental compilation */
    "target": "ES2015",                          /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', or 'ESNEXT'. */
    "module": "commonjs",                     /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', 'es2020', or 'ESNext'. */
    // "lib": [],                             /* Specify library files to be included in the compilation. */
    // "allowJs": true,                       /* Allow javascript files to be compiled. */
    // "checkJs": true,                       /* Report errors in .js files. */
    "jsx": "preserve",                     /* Specify JSX code generation: 'preserve', 'react-native', or 'react'. */
    // "declaration": true,                   /* Generates corresponding '.d.ts' file. */
    // "declarationMap": true,                /* Generates a sourcemap for each corresponding '.d.ts' file. */
    // "sourceMap": true,                     /* Generates corresponding '.map' file. */
    // "outFile": "./",                       /* Concatenate and emit output to single file. */
    // "outDir": "./",                        /* Redirect output structure to the directory. */
    // "rootDir": "./",                       /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */
    // "composite": true,                     /* Enable project compilation */
    // "tsBuildInfoFile": "./",               /* Specify file to store incremental compilation information */
    // "removeComments": true,                /* Do not emit comments to output. */
    "noEmit": true,                        /* Do not emit outputs. */
    // "importHelpers": true,                 /* Import emit helpers from 'tslib'. */
    // "downlevelIteration": true,            /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */
    // "isolatedModules": true,               /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */

    /* Strict Type-Checking Options */
    "strict": false,                           /* Enable all strict type-checking options. */
    // "noImplicitAny": true,                 /* Raise error on expressions and declarations with an implied 'any' type. */
    // "strictNullChecks": true,              /* Enable strict null checks. */
    // "strictFunctionTypes": true,           /* Enable strict checking of function types. */
    // "strictBindCallApply": true,           /* Enable strict 'bind', 'call', and 'apply' methods on functions. */
    // "strictPropertyInitialization": true,  /* Enable strict checking of property initialization in classes. */
    // "noImplicitThis": true,                /* Raise error on 'this' expressions with an implied 'any' type. */
    // "alwaysStrict": true,                  /* Parse in strict mode and emit "use strict" for each source file. */

    /* Additional Checks */
    // "noUnusedLocals": true,                /* Report errors on unused locals. */
    // "noUnusedParameters": true,            /* Report errors on unused parameters. */
    // "noImplicitReturns": true,             /* Report error when not all code paths in function return a value. */
    // "noFallthroughCasesInSwitch": true,    /* Report errors for fallthrough cases in switch statement. */

    /* Module Resolution Options */
    // "moduleResolution": "node",            /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */
    // "baseUrl": "./",                       /* Base directory to resolve non-absolute module names. */
    // "paths": {},                           /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */
    // "rootDirs": [],                        /* List of root folders whose combined content represents the structure of the project at runtime. */
    // "typeRoots": [],                       /* List of folders to include type definitions from. */
    // "types": [],                           /* Type declaration files to be included in compilation. */
    // "allowSyntheticDefaultImports": true,  /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */
    "esModuleInterop": true,                  /* Enables emit interoperability between CommonJS and ES Modules via creation of namespace objects for all imports. Implies 'allowSyntheticDefaultImports'. */
    // "preserveSymlinks": true,              /* Do not resolve the real path of symlinks. */
    // "allowUmdGlobalAccess": true,          /* Allow accessing UMD globals from modules. */

    /* Source Map Options */
    // "sourceRoot": "",                      /* Specify the location where debugger should locate TypeScript files instead of source locations. */
    // "mapRoot": "",                         /* Specify the location where debugger should locate map files instead of generated locations. */
    // "inlineSourceMap": true,               /* Emit a single file with source maps instead of having a separate file. */
    // "inlineSources": true,                 /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */

    /* Experimental Options */
    // "experimentalDecorators": true,        /* Enables experimental support for ES7 decorators. */
    // "emitDecoratorMetadata": true,         /* Enables experimental support for emitting type metadata for decorators. */

    /* Advanced Options */
    "skipLibCheck": true,                     /* Skip type checking of declaration files. */
    "forceConsistentCasingInFileNames": true  /* Disallow inconsistently-cased references to the same file. */
  }
}

Environment (optional)

  • Browser and Version: XXX
  • OS: XXX
  • Node Version: XXX
  • Package Manager and Version: XXX
@kumar303
Copy link
Contributor Author

Oh! Is this what it should be?

// Common use-case
const add = (amount: number) => ({ type: 'ADD' as 'ADD', payload: amount });
type AddAction = $Call<typeof add>; // { type: 'ADD'; payload: number }

@piotrwitek
Copy link
Owner

Oh! Is this what it should be?

Yes, it's a typo in docs, PR is welcomed!

Regarding the OP issue, it's impossible to implement the same behavior using TS as it's used in Flow at the moment, so we are using a simpler alternative.

Maybe the solution here is to change the name/description so it's less confusing. Feel free to suggest something better and open a PR. I'll be happy to help and the community will benefit from it.

kumar303 added a commit to kumar303/utility-types that referenced this issue Sep 11, 2020
See piotrwitek#149 . I don't know if this patch should close that issue, though.
@kumar303
Copy link
Contributor Author

kumar303 commented Sep 11, 2020

Oh! Is this what it should be?

Yes, it's a typo in docs, PR is welcomed!

Ah, great. I just edited on the GitHub site: #154

Regarding the OP issue, it's impossible to implement the same behavior using TS as it's used in Flow at the moment, so we are using a simpler alternative.

Maybe the solution here is to change the name/description so it's less confusing. Feel free to suggest something better and open a PR. I'll be happy to help and the community will benefit from it.

Hmm, I'm not sure. I guess it's fine. I was converting from Flow so I ended up just going with the built-in ReturnType in most places instead of $Call because it handled union types better. I could link to ReturnType in the docs if you'd like.

You may be aware already but flow-to-ts automatically converts to $Call from utility-types.

piotrwitek pushed a commit that referenced this issue Sep 12, 2020
See #149 . I don't know if this patch should close that issue, though.
@piotrwitek
Copy link
Owner

Thanks for the typo fix!

I  think adding the comment and linking to build-in ReturnType would be great for inexperienced users 👍 Basically they should work the same.

@kumar303
Copy link
Contributor Author

I think adding the comment and linking to build-in ReturnType would be great for inexperienced users 👍 Basically they should work the same.

Sure. I proposed this #155

@piotrwitek
Copy link
Owner

I think this can be closed now.

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

2 participants