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

feat: bundle type definitions into a single file per module #12345

Merged
merged 2 commits into from
Feb 9, 2022

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Feb 9, 2022

Summary

This should

  1. ensure we only give access to exported types from packages
  2. decrease install time as the tarball is smaller and there are less files to put onto disk
  3. decrease tsc time (for consumers) as tsc only needs to read a single file per module instead of following a bunch of imports

Test plan

This is the entire diff of all packages/build/**/*d.ts: https://gist.github.com/SimenB/4f583a1a737b913a0328985bbf8df617

While the diff is 329 files changed, 5356 insertions(+), 9134 deletions(-), there are 280 deleted files.

I'll probably land this, then test it more thoroughly with a beta release of v28

@codecov-commenter
Copy link

Codecov Report

Merging #12345 (e2e5f95) into main (7f7b3fb) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #12345   +/-   ##
=======================================
  Coverage   66.97%   66.97%           
=======================================
  Files         329      329           
  Lines       17335    17335           
  Branches     5061     5061           
=======================================
  Hits        11610    11610           
  Misses       5693     5693           
  Partials       32       32           

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 7f7b3fb...e2e5f95. Read the comment docs.

@mrazauskas
Copy link
Contributor

Impressive!

What if some basic type tests would be introduced for package with exported types? Just to cover exports which must be available for consumers. For instance, current type test in expect is importing types from the build. Have to double check, but I think the test also makes sure that these types are actually exported. Should be so.

I could try to cover at least the most important packages. This should give more confidence + will be helpful in CI for the bundled future.

What you think? And if this sounds helpful, what’s the approx timeframe? (;

(12345 – magic number! Special PR get special numbers.)

@SimenB
Copy link
Member Author

SimenB commented Feb 9, 2022

The type tests run after bundling I think (or, at least that was my intention). If not I'll fix that

Adding more tests would be great, though! 😃

@SimenB
Copy link
Member Author

SimenB commented Feb 9, 2022

Yep, type tests run after bundle, so any tests we have run against the bundled definitions.

https://github.com/facebook/jest/blob/e9eced4ffd21b6fb33d01867315fcefeae261b15/.github/workflows/nodejs.yml#L58-L61

No idea why all windows builds failed, tho....

@SimenB SimenB merged commit 819c2be into jestjs:main Feb 9, 2022
@SimenB SimenB deleted the api-extractor branch February 9, 2022 20:31
@mrazauskas
Copy link
Contributor

No idea why all windows builds failed, tho....

While playing with #12327, I noticed that Windows tests run on Windows Server 2022. Just a week ago it was Windows Server 2019. This morning a newsletter from GitHub came confirming that they are migrating windows-latest runner.

Might be this is the reason I couldn’t make the flaky test fail. Perhaps it is not flaky anymore and that’s good. Now two other SCM tests are failing from time to time on Windows. Possibly just a timeout, because of slower FS or so. I keep an eye on these (;

@SimenB
Copy link
Member Author

SimenB commented Feb 10, 2022

merging in main made them all pass, so who knows! 😀

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants