atlassian-labs / compiled

A familiar and performant compile time CSS-in-JS library for React.
https://compiledcssinjs.com
Apache License 2.0
1.99k stars 67 forks source link

Missing type checking in tests #797

Open MonicaOlejniczak opened 3 years ago

MonicaOlejniczak commented 3 years ago

Describe the bug Compiled uses TypeScript project references, which unfortunately prevents consumers from only type checking via tsc --noEmit or ttsc --noEmit. In order to determine whether the codebase has type errors, the entire project needs to be built (refer to https://github.com/microsoft/TypeScript/issues/40431 and https://www.typescriptlang.org/docs/handbook/project-references.html):

tsc will not automatically build dependencies unless invoked with the --build switch

Therefore, the main type checking is performed through the yarn build command. However, test files are explicitly excluded through the shared config as seen here. This means that potential type errors and problematic cases are missed in the tests, including what is meant to be type tests (e.g. should not type error with nested arrow funcs is not properly asserted), invalid parameters (e.g. { minify: true }) are not detected, missing type dependencies, etc.

To Reproduce Steps to reproduce the behaviour:

  1. Add the following line to a test file: const invalidNumber: number = 'not a number';
  2. Run yarn build — notice there are no errors
  3. Add the following line to a source file: const invalidBoolean: boolean = 'not a boolean';
  4. Run yarn build — notice there are now type errors

Expected behaviour Running yarn build (or specific type checking script) should result in a type error within a test file.

Notes

itsdouges commented 3 years ago

I think the current setup with Jest will error tests if there are type errors, if that gives you better confidence 🤔

Have a double check 😄, hopefully I remember correctly.

MonicaOlejniczak commented 3 years ago

@madou Running yarn test packages/react/src/styled/__tests__/index.test.tsx from this:

it('should not type error', () => {
  expect(() => {
    styled.div<{ primary: string }>({
      fontSize: '20px',
      color: (props) => props.primary,
      margin: '20px',
      ':hover': {
        color: 'red',
      },
    });
  }).not.toThrow();
});

to this, which removes the generic, has no effect on the test output:

it('should not type error', () => {
  expect(() => {
    styled.div({
      fontSize: '20px',
      color: (props) => props.primary,
      margin: '20px',
      ':hover': {
        color: 'red',
      },
    });
  }).not.toThrow();
});

The expected type error is, which is produced by including the test files:

packages/react/src/styled/__tests__/index.test.tsx:271:27 - error TS2571: Object is of type 'unknown'.

271         color: (props) => props.primary,

Though my editor integration (IntelliJ) has a slightly different error:

TS7006: Parameter 'props' implicitly has an 'any' type.
itsdouges commented 2 years ago

@MonicaOlejniczak I think this might be fixed now thanks to the PR I put up refactoring the local dev loop.