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

Updating And Improvement Typescript Infrastructure #65

Closed

Conversation

sgrishchenko
Copy link
Contributor

@sgrishchenko sgrishchenko commented Jan 19, 2019

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

feature
infrastructure

What is the current behaviour? (You can also link to an open issue here)

  1. now typings file contains over 1500 LOC
  2. now library supports only homogeneous selectors

What is the new behaviour?

  1. all boilerplate code of createCachedSelector overloads will be generated
  2. homogeneous selectors replased with heterogeneous (they completely cover all cases)
  3. added type checking of generated typings with tsc --noEmit

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

After the updates I corrected the tests a little, so I think it will be breaking changes for typescript users.

Other information:

CI workflow changed (now types are being tested after compilation, because they are generated)

Please check if the PR fulfills these requirements:

  • Tests for the changes have been added
  • Docs have been added / updated

@coveralls
Copy link

coveralls commented Jan 19, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling a725853 on sgrishchenko:type-generator into a563ac8 on toomuchdesign:master.

@toomuchdesign
Copy link
Owner

Hi @sgrishchenko,
thanks for your PR's!

I checked out this PR, PR #63 and what's going on on Reselect repo.

From my point of view I agree we should definitely update TS typings.

Even though I find brillant the idea of dynamically generating the typings, I'm a bit reluctant to add such a tick layer of TS toolings on top of current setup.

I'd like to merge PR #63 (old style hardocoded typings), take some time to see if those TS typing tooling get more "mainstream" (I'll keep an eye on your PR on reselect 😄repo), and in case replicate the existing typings with this PR.

How do you feel about this plan?
Thank's again. Greetings!

@sgrishchenko
Copy link
Contributor Author

As you can see, I VERY much want heterogeneous selectors to be in your library 😄. I will be very happy if you merge my PR #63 🎉.

As for code generation, I will be frank: it seems to me that it will take a very long time before my changes are noticed in the original Reselect repo. This approach has never been mainstream and is unlikely to be. In your hands try to change it 😉...

About "TS toolings": I installed only 4 libraries. mkdirp and ncp these are just tools for creating directories and copying files, there is no specifics for typescript in them. It remains ts-simple-ast and ts-node. ts-simple-ast is used to work with Typescript AST and generate code. I could use just template strings, but it seems to me less elegant and easier to get hurt. ts-node is used to run Typescript file on the Node.js. Again, I could just write code in javascript, but I thought that when working with AST, type checking would be useful for reliability. It turns out that I use only two libraries for the typescript. In my opinion, not so much, right 😄😉? Seriously, I just tried to tell in more detail what and why I used, so that you have less concern about the introduction of a new approach.

Thanks so much for the quick response and feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants