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

Switched to using dtslint from just running tsc --noEmit for typescript tests #421

Merged
merged 6 commits into from Nov 1, 2017

Conversation

cameron-martin
Copy link
Contributor

@cameron-martin cameron-martin commented Oct 19, 2017

What:
Changed the TypeScript tests to using dtslint.

Why:

  • Most importantly it lets you use $ExpectError to assert that a certain statement does type error, which the previous type tests did not.
  • It also tests against multiple TypeScript versions.

How:
By changing from running tsc --noEmit to running dtslint for running the TypeScript tests.

Checklist:

  • Documentation
  • Tests
  • Code complete

I'm having trouble running the dtslint tests for the react-emotion package - some of the features used in the dts files require a new version of TypeScript but I keep getting an error (microsoft/dtslint#78) when trying to use new versions of TypeScript.

This PR is just to make sure you agree with this direction before I pursue it even more and to collaborate if anyone else is working on it.

A few related changes, some as a side-effect of moving to dtslint:

  • Made some changes to the dts files to satisfy the default dtslint rules.
  • Renamed typings directory to types because this seems to be more standard.
  • Renamed emotion.d.ts, etc to index.d.ts because dtslint seems to require this.
  • Stopped "build" running before "test:typescript" as this is not actually necessary.

@renatorib
Copy link
Contributor

renatorib commented Oct 20, 2017

That's great!

I saw that you renamed some names, like typings => types, react-emotion.d.ts => index.d.ts, etc.
I think this is not a good idea since all other emotion packages follows this names/structure.

Or, if you think this names/structure is better or necessary for that approach, you could rename from all other emotion packages also to maintain a consistency.

Edit: Sorry, I just saw that you already changed the other packages too

@cameron-martin
Copy link
Contributor Author

I think dtslint requires that the dts file is named index.d.ts (although I could be wrong about this, I'll check). But yeah I'll make sure everything is consistent.

…pt tests.

A few related changes, some as a side-effect of doing so:
* Renamed typings directory to types because this seems to be more standard.
* Renamed emotion.d.ts, etc to index.d.ts because dtslint seems to require this.
* Stopped "build" running before "test:typescript" as this is not actually necessary.
@cameron-martin
Copy link
Contributor Author

cameron-martin commented Oct 24, 2017

The new version of dtslint has fixed those errors I was getting. I'm just getting a couple of errors with typescript@next (https://travis-ci.org/emotion-js/emotion/builds/292255135#L681), then this'll be done.

It's because of strictFunctionTypes, which is being introduced in TypeScript 2.6, which has got something to do with variance.

Any ideas, besides just turning off this flag?

@cameron-martin
Copy link
Contributor Author

The above problem is now fixed.

@cameron-martin cameron-martin changed the title WIP: Switched to using dtslint from just running tsc --noEmit for typescript tests Switched to using dtslint from just running tsc --noEmit for typescript tests Oct 25, 2017
@tkh44 tkh44 merged commit 78f2ae9 into emotion-js:master Nov 1, 2017
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

Successfully merging this pull request may close these issues.

None yet

4 participants