-
-
Notifications
You must be signed in to change notification settings - Fork 835
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
Update tsconfig file to include .tsx files #2457
Update tsconfig file to include .tsx files #2457
Conversation
I edited the description so the issue isn't closed if we merge this PR. |
js/tsconfig.json
Outdated
@@ -1,5 +1,5 @@ | |||
{ | |||
"include": ["src/**/*.ts"], | |||
"include": ["src/**/*"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels quite broad, essentially saying any file in src. Couldn't this cause files unrelated to JS/TS/TSX to be recognized incorrectly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@askvortsov1, according to the official documentation, this will actually do what we want it to do - parse only TypeScript files:
If a glob pattern doesn’t include a file extension, then only files with supported extensions are included (e.g. .ts, .tsx, and .d.ts by default, with .js and .jsx if allowJs is set to true).
https://www.typescriptlang.org/tsconfig#include
Since allowJs is not explicitly set to true, this change will mean that TS will go from parsing just .ts
files to .ts
, .tsx
, and d.ts
, ignoring JS files and anything else that may be present in those directories.
Thanks for approving this @askvortsov1. I was thinking that perhaps it would be better to set the files to parse explicitly instead of following the TS docs - that way this config value would essentially become self-documenting:
|
Ah yes that's even better! |
@askvortsov1 done! |
Refs #3533
Doesn't really fix the umbrella issue but is related to it.
Changes proposed in this pull request:
Change
tsconfig.json
so that project-specific configuration is taken into account not just for.ts
files but for.tsx
files as well. Removing the file extension makes TS default to processing just.ts
,.tsx
, and.d.ts
. files.Confirmed
Required changes: