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

Improve documentation #55

Open
SamVerschueren opened this issue Nov 13, 2019 · 12 comments
Open

Improve documentation #55

SamVerschueren opened this issue Nov 13, 2019 · 12 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@SamVerschueren
Copy link
Collaborator

I think we need to improve the documentation a bit. But I'm not sure how to provide the right amount of information.

So any ideas here are more than welcome. Like, should we add more examples? Should we add an example per assertion to make it more clear? Is the description good enough?

@SamVerschueren SamVerschueren added help wanted Extra attention is needed question Further information is requested labels Nov 13, 2019
@treybrisbane
Copy link

A few ideas off the top of my head:

  • Clarity around intended use cases (I mistakenly thought this was a library for testing .ts files rather than .d.ts files)
  • A dedicated section on configuration (I ended up looking through the code to find the possible config); this should ideally include:
    • The set of all possible configuration options
    • Short descriptions for each option
    • The set of possible values (i.e. the type) of each option
    • For any optional options, the default value (I'm so sad when libraries don't include this!)
  • Slightly restructure the example/quick start guide to show how things fit into an actual project; e.g. improve clarity on things like:
    • "Let's assume we wrote an index.d.ts" - okay, where is this file? My project root? A dist folder?
    • "Add an index.test-d.ts" - okay, where? Same directory as the index.d.ts? In a test directory?

Hope that helps! 🙂

@papb
Copy link
Contributor

papb commented Apr 19, 2020

Hi everyone, I am trying to learn how to use this tool, but to be honest I find the documentation very lacking. I see that Sindre uses this in basically all his projects and it seems great, but really I can't figure out how expectError works, at all. I was about to open an issue but then I saw this one.


Let me begin by quoting readme:

expectError<T>(value)

Check if a value is of the provided type T.

There must be some mistake here in the readme, right? This explanation does not even seem to be connected with the name expectError.


expectError(function)

Check if the function call has argument type errors.

Although this is definitely less cryptic than the explanation for expectError<T>, it must be better worded somehow, or maybe an example should be given. Also, apparently passing or not a <T> to the expectError call makes a huge difference, so this should be highlighted.


Long story short, I had to install tsd and play with it manually to begin understanding what is happening. And I still don't. I will be updating here when I make more discoveries.

@papb
Copy link
Contributor

papb commented Apr 19, 2020

I will include here a summarized version of my thought process and experimentation process, hoping it will help clarify in which sense I think the docs are lacking.

My setup:

  • npm init -y && npm i -D tsd

  • index.d.ts:

    declare const concat: {
    	(value1: string, value2: string): string;
    	(value1: number, value2: number): string;
    };
    
    export default concat;
  • index.test-d.ts:

    import {expectType, expectError} from 'tsd';
    import concat from '.';
    
    // See my next comments for what I have put here
  • npx tsd

@papb
Copy link
Contributor

papb commented Apr 19, 2020

  • The following passes:

    expectError(concat(5, 's'));
    expectError<string>(concat(5, 's'));
    expectError<void>(concat(5, 's'));
    expectError<number>(concat(5, 's'));
    expectError<any>(concat(5, 's'));
    expectError<Error>(concat(5, 's'));
    expectError<(typeof expectError)[]>(concat(5, 's'));
    expectError<unknown>(concat(5, 's'));
  • However the following fails:

    expectError<never>(concat(5, 's'));

So far I have no idea what is this T type doing.

@papb
Copy link
Contributor

papb commented Apr 19, 2020

Also, my editor shows a red squiggle below all concat calls, which is OK, and interesting to notice that these errors do not crash tsd. So this means tsd is doing some amazing magic under the hood? Looks like tsd is not just another tool. Maybe it is doing some AST magic?

Update: adding console.log('TEST'); did not show anything. Maybe the code isn't even being executed?
Update 2: definitely not being executed, I tried to write to a file as well but didn't work. Interesting.

@papb
Copy link
Contributor

papb commented Apr 19, 2020

  • The following fails with two errors:
expectError<boolean>(someIdentifierNotEvenDeclared);

× 4:0 Expected an error, but found none.
× 4:21 Cannot find name someIdentifierNotEvenDeclared.

🤔 I have no idea what is happening here. It found an error but didn't.

  • I decided to try a random syntax error as well:
expectError({{}});

× 4:13 Property assignment expected.
× 4:15 , expected.

Maybe expectError is more like expectErrorExceptSyntaxError?

@papb
Copy link
Contributor

papb commented Apr 19, 2020

Oh! The following does not pass:

const res = concat(5, 's');
expectError(res);

× 4:12 No overload matches this call.
× 5:0 Expected an error, but found none.

So it looks like tsd is doing some deep magic with the source code itself. Like parsing the AST and statically analyzing what is literally inside the parenthesis from the expectError call?

@papb
Copy link
Contributor

papb commented Apr 19, 2020

Sorry for flooding the thread. Let me know if I should delete the above comments.


Request summary

  • Somewhere at the beginning of the readme, mention something like:

    tsd does not execute your .test-d.ts file(s). It does not even compile your .test-d.ts file(s) like a normal typescript file. What it does is some heavy witchcraft on your .test-d.ts file(s) source code directly, which happens to be written in a typescript-like syntax, and detects specific syntactical constructs such as expectError(foo) and handles them automagically. Since these files have a ts extension, your IDE might even "detect" errors in a file which is in fact, a perfectly valid .test-d.ts file.

  • Fix the explanation for expectError<T>(value) in readme, if it's not wrong it's very unclear.

  • Highlight the difference between expectError and expectError<T>

  • Provide lots of examples for expectError

@andrewsantarin
Copy link

Adding up to @treybrisbane's #55 (comment) with regards to test file placement:

I've just started using tsd. I'm very new to this tool and TS testing in general. Getting this set up for the library I'm trying to augment with TS drove me nuts for a couple of hours!

tsd's documentation expects the .test-d.ts to be at the root folder:

node_modules
index.js
index.d.ts
index.test-d.ts
package.json
tsconfig.json

my-would-be-typed-libary uses a src-like structure, with the conventional typings/types folder I've added to augment the .js files, whose structure I simply derived from the original library to keep things simple to understand:

node_modules
src
  components
    __tests__
      JsTest1.js
      JsTest2.js
  index.js
types
  components
    __tests__
      TsTest1.test-d.ts
      TsTest2.test-d.ts
  index.d.ts
package.json
tsconfig.json

As you can see, I can't just plonk an index.d.ts in the root without reconfiguring the whole OSS library.

Being the novice that I am, I imagined I'd find the clues in the README file. I couldn't find it.

Thankfully, I found myself a reference from @sindresorhus's works. Otherwise, I'd still be stuck. To support my working structure, I must use this setup:

package.json

{
  "name": "my-would-be-typed-libary",
  "types": "types/index.d.ts",
  "tsd": {
    "directory": "types/components/__tests__"
  },
  ...
}

@papb
Copy link
Contributor

papb commented Jul 2, 2020

@SamVerschueren If you could please provide answers to my questions here, I will be happy to send a PR clarifying everything. Thank you!

@BendingBender
Copy link
Collaborator

BendingBender commented May 24, 2021

@papb I probably can answer some of your questions. Some things that you've found are bugs, some are intended behavior.

First of all, tsd doesn't execute your code under test, this is not a test runner, it is a tool to allow you to make assertions for your type definitions. The definitions are checked by spinning up the TypeScript compiler, extracting all the errors that the compiler finds and reporting them (with some additional magic for the expect... functions). To make it clear: at no time these functions are actually ever executed, they are simply used for the TypeScript compiler to check your types.

  • The following passes:

    expectError(concat(5, 's'));
    expectError<string>(concat(5, 's'));
    expectError<void>(concat(5, 's'));
    expectError<number>(concat(5, 's'));
    expectError<any>(concat(5, 's'));
    expectError<Error>(concat(5, 's'));
    expectError<(typeof expectError)[]>(concat(5, 's'));
    expectError<unknown>(concat(5, 's'));
  • However the following fails:

    expectError<never>(concat(5, 's'));

So far I have no idea what is this T type doing.

You have multiple things going on here, you've triggered the bugs fixed in #103 and #104.

expectError is meant to silence type erros in the expression passed to it. The variant of expectError with a type parameter T is meant to assert that we expect a type error when trying compare the types returned from the expression passed to expectError and the type parameter.

The way expectError is currently implemented, it will not differentiate between the kinds of errors that you have in the expression passed to it. In the examples above you have multiple errors in your expression. You pass invalid arguments to concat and compare the return type of concat(5, 's') with the different types passed to expectError<T>. expectError will assume that if you have any error in your expression then the test will pass.

So it's up to you to make sure that you don't have any other type errors in your expression for expectError than the one that you actually want to test for. For your case, the approriate tests should look like the following (which will work as intended as soon as #104 is merged):

expectError(concat(5, 's')); // passes (after #104), expression contains a type error (no overload signature found...)
expectError<string>(concat(5, 5));  // fails, no error, result of `concat` is assignable to `string`
expectError<void>(concat(5, 5)); // passes, result of `concat` is not assignable to `void`
expectError<number>(concat(5, 5)); // passes, result of `concat` is not assignable to `number`
expectError<any>(concat(5, 5)); // fails, everything is assignable to `any`
expectError<Error>(concat(5, 5)); // passes, result of `concat` is not assignable to `Error`
expectError<typeof expectError[]>(concat(5, 5)); // passes, result of `concat` is not assignable to `typeof expectError[]`
expectError<unknown>(concat(5, 5)); // fails, everything is assignable to `unknown`
expectError<never>(concat(5, 5)); // passes, result of `concat` is not assignable to `never`
  • The following fails with two errors:
expectError<boolean>(someIdentifierNotEvenDeclared);

× 4:0 Expected an error, but found none.
× 4:21 Cannot find name someIdentifierNotEvenDeclared.

thinking I have no idea what is happening here. It found an error but didn't.

  • I decided to try a random syntax error as well:
expectError({{}});

× 4:13 Property assignment expected.
× 4:15 , expected.

Maybe expectError is more like expectErrorExceptSyntaxError?

Yes, exactly, expectError is meant to only catch type errors. Syntax errors are excluded on purpose, why would you want to have syntax errors in your test code? It doesn't really make sense, that's why they are excluded.

We extend expectError with the errors that it has to ignore continuously because the TypeScript compiler has a vast amount of diagnostic messages it produces (see here) and we add the ones that make sense to silence for expectError one by one based on issue reports from users.

Hope this helps and clarifies a few things.

@BendingBender
Copy link
Collaborator

As an aside, you're very welcome to make PRs for documentation improvements.

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 question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants