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

test: ensure return type of functions #681

Open
johnhooks opened this issue Nov 28, 2022 · 3 comments · May be fixed by #767
Open

test: ensure return type of functions #681

johnhooks opened this issue Nov 28, 2022 · 3 comments · May be fixed by #767
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@johnhooks
Copy link
Contributor

johnhooks commented Nov 28, 2022

There was an odd issue in PR #309, where a function returned unknown rather than the expected type but it didn't raise any type errors and passed all the tests.

There must be a standard way to test types, and prevent this in the future.

@johnhooks
Copy link
Contributor Author

johnhooks commented Nov 28, 2022

It would be nice if something like this worked. It does show an error in the editor and doesn't pass the script test:types, but test passes because Jest strips all the types before preforming a test run.

// idea for packages/dinero.js/src/api/__tests__/toSnapshot.test.ts in branch feat/to-units
describe('custom transformer', () => {
  const amount = new Big(500);
  const d = dinero({ amount, currency: bigjsUSD });
  const expected = toSnapshot(d, ({ value }) => value.amount);

  it('returns the expected value', () => {
    expect(expected).toBe(amount);
  });

  it('returns the expected type', () => {
    expect(() => {
      // @ts-expect-no-error
      const typed: Big = expected;
    }).not.toThrow();
  });
});

@johnhooks johnhooks changed the title test: ensure return type of the functions test: ensure return type of functions Nov 28, 2022
@sarahdayan
Copy link
Collaborator

It's a good idea to add return types to functions. Initially I disabled it because I thought inference was better, but explicit return types ensure we don't accidentally break the API or that we don't have unexpected return types as in the example you shared.

To do this, we need to:

  • Re-enable this rule
  • Add explicit return types to each function (once the ESLint rule is re-enabled, running yarn lint) should report all changes to make

@sarahdayan sarahdayan added enhancement New feature or request good first issue Good for newcomers labels Nov 29, 2022
@aushwin
Copy link

aushwin commented Jan 27, 2023

Hi @sarahdayan am new to contributions, can I try to work on this ?

@ch1booze ch1booze linked a pull request Jul 12, 2024 that will close this issue
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants