Navigation Menu

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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the types in shipped package #1842

Merged
merged 2 commits into from Aug 12, 2021
Merged

Fix the types in shipped package #1842

merged 2 commits into from Aug 12, 2021

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Aug 12, 2021

1829 added the spec files to 'src', presumably because they just
were't type checked otherwise, but this chaqnged the implicit rootDir
so the lib directory we shipped has types in src/ and spec/ which
meant lib/index.d.ts was at lib/src/index.d.ts.

Specify the rootDir on the build tsconfig to put it back in the
right place, and also try to make the base tsconfig never emit
to avoid it ever generating .d.ts files in silly places. Move all
the options related to emitting to -build since they aren't relevant
in the main one.

Is this a sensible way to do this? It doesn't feel terribly elegant.

Fixes element-hq/element-web#18503
Regressed in #1829


Here's what your changelog entry will look like:

馃悰 Bug Fixes

1829 added the spec files to 'src', presumably because they just
were't type checked otherwise, but this chaqnged the implicit rootDir
so the lib directory we shipped has types in src/ and spec/ which
meant lib/index.d.ts was at lib/src/index.d.ts.

Specify the rootDir on the build tsconfig to put it back in the
right place, and also try to make the base tsconfig never emit
to avoid it ever generating .d.ts files in silly places. Move all
the optoins related to emitting to -build since they aren't relevant
in the main one.

Is this a sensible way to do this? It doesn't feel terribly elegant.

Fixes element-hq/element-web#18503
Regressed in #1829
@dbkr dbkr added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Aug 12, 2021
@dbkr dbkr requested a review from a team as a code owner August 12, 2021 10:51
@Palid
Copy link
Contributor

Palid commented Aug 12, 2021

@dbkr I think in this case the easiest solution would be to make a base.tsconfig.json somewhere else (like we have with eslint?) and just extend it everywhere we need.
Current solution isn't bad, but I wouldn't remove all those compiler options from the tsconfig.json, as it may break type reporting if you were to run typescript:watch task in vscode.

@dbkr
Copy link
Member Author

dbkr commented Aug 12, 2021

Yeah, a tsconfig shared between projects would definitely make sense, but that's not a battlefront I want to open up right now, and isn't really the issue here? Only options being removed now are emitDecoratorMetadata are sourcemap which shouldn't matter unless we're emitting stuff? My question was more if this was a sensible way to tell typescript, "hey, these test files, they exist, please check them, but don't emit compiled code or types for them".

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

This is unfortunately the way you do this on TS.

@dbkr
Copy link
Member Author

dbkr commented Aug 12, 2021

Fair enough :/

@dbkr dbkr merged commit 1298ed6 into release-v12.3.0 Aug 12, 2021
@dbkr dbkr deleted the dbkr/fix_types_build branch August 12, 2021 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants