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

Discussion: Some PR I would like to make #178

Closed
Mister-Hope opened this issue Jan 9, 2021 · 6 comments
Closed

Discussion: Some PR I would like to make #178

Mister-Hope opened this issue Jan 9, 2021 · 6 comments

Comments

@Mister-Hope
Copy link
Contributor

Mister-Hope commented Jan 9, 2021

First of all, I would like to request opening the discussion panel.

I thinks putting issues with content like this to discssion panel would be better.

And I am planning to submit these pr:

And I like to request this changes:

  1. Add prettier to this repo and use prettier style (I will also import eslint-config-prettier to make sure no conflict with eslint)

Reason: 4 space indent make the type files hard to read( especially in complex interface), And prettier do have a better wrap lint behavior.

Before:
image

After:
image

I think below is better.

  1. Rewrite the whole type test using jest with ts-jest.

We are just using a simple type:

const expectType: <T>(value: T) => void;

Moving to jest can have better test and error output and scope handle(with describe and it)

  1. migrate to github actions.

Reasons: More convenient to open action logs. Besides, visiting travis is slow in China.

I am looking forward your opinion towards the 3 changes above. If you thinks those are fine, I will make PRs

And also I wanna know whether it's possible to add generics in apis (I am not sure if it can be done with auto generation) Related request are #138, #177

@Mister-Hope
Copy link
Contributor Author

image
Also I just found a werid thing

@SgLy
Copy link
Contributor

SgLy commented Jan 11, 2021

  1. Rewriting Component types or adding tests are always welcomed, feel free to make PRs;
  2. Prettier is great, but a tab width of 4 is to meet DefinitelyTyped's requirement; using their prettier configuration maybe a better solution;
  3. <T>(value: T) => void is weaker than the one tsd provided: this only checks if value can be assigned to T, while tsd's expectType checks if the two type are identical. As an example:
    declare const a: any;
    expectType<number>(a)
    will pass <T>(value: T) => void but fails tsd;
  4. Tests should have been formatted by eslint. This will be fixed soon.

@Mister-Hope
Copy link
Contributor Author

Mister-Hope commented Jan 11, 2021

Prettier is great, but a tab width of 4 is to meet DefinitelyTyped's requirement; using their prettier configuration maybe a better solution;
Tests should have been formatted by eslint. This will be fixed soon.

I'll do the jobs by sending PR. I am already working on it.

@Mister-Hope
Copy link
Contributor Author

Just to confirm:
4 spaces in types to met DefinitelyTyped's requirement and 2 space in test files right?

@Mister-Hope
Copy link
Contributor Author

And what about this one?

migrate to github actions.
Reasons: More convenient to open action logs. Besides, visiting travis is slow in China.

@SgLy
Copy link
Contributor

SgLy commented Jan 11, 2021

Just to confirm:
4 spaces in types to met DefinitelyTyped's requirement and 2 space in test files right?

I prefer to just format test cases using the same configuration with types and made it 4 space indented, for not mixing different tab widths in a single project.

And what about this one?

migrate to github actions.
Reasons: More convenient to open action logs. Besides, visiting travis is slow in China.

Migrating to GItHub Actions is also in my plan (but with a low precedence), a PR will be great.

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