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

Create types using tsconfig.json #183

Merged
merged 1 commit into from
Sep 2, 2021
Merged

Create types using tsconfig.json #183

merged 1 commit into from
Sep 2, 2021

Conversation

esanzgar
Copy link
Contributor

@esanzgar esanzgar commented Sep 2, 2021

Problem: the tsc command that generate the type declaration didn't
include all the options used for type checking the project.

Solution: use tsc --build command for both, type checking, and the
generation of the type declaration.

Problem: the `tsc` command that generate the type declaration didn't
include all the options used for type checking the project.

Solution: use `tsc --build` command for both, type checking, and the
generation of the type declaration.
@codecov
Copy link

codecov bot commented Sep 2, 2021

Codecov Report

Merging #183 (7502d2c) into main (829d639) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #183   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines          224       224           
  Branches        75        75           
=========================================
  Hits           224       224           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 829d639...7502d2c. Read the comment docs.

@esanzgar esanzgar requested a review from lyzadanger September 2, 2021 15:01
Copy link
Contributor

@lyzadanger lyzadanger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that building the package with these settings fixes the problematic typing for the selectedItem prop on the Table component, and that linking my local lms branch against a build of this eliminates the TS error I was encountering earlier today on that prop.

I'm not certain if there are other things I should keep an eye out for, or other risks raised by this change, but from what I can see here, it looks like it does what it is supposed to!

@esanzgar
Copy link
Contributor Author

esanzgar commented Sep 2, 2021

The only minor difference regarding this change is that checking the types with make lint will generate type declarations. Before this change, it didn't do that. I tried to avoid that but I couldn't easily, but I think we can live with that.

There is actually a way to avoid generating type declaration when checking the types. It involves having two tsconfig.json one that extends from the other. I think the previous solution is good enough.

@esanzgar esanzgar merged commit afd1d8f into main Sep 2, 2021
@esanzgar esanzgar deleted the types-using-tsconfig branch September 2, 2021 16:10
esanzgar added a commit that referenced this pull request Sep 2, 2021
Added a new `tsconfig.json` file that overrides some of the options of
`tsconfig` and generates the type declarations.
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.

2 participants