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

fix: downlevel .d.ts files for compat with older TS versions #9705

Merged
merged 4 commits into from
Mar 26, 2020

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Mar 26, 2020

Summary

We shouldn't force consumers to update to TS 3.8. This will downlevel the d.ts files so it works down to TS 3.4.

Fixes #9703
Closes #9704

Test plan

I'll publish a canary and try to build with an older version of TS.

EDIT: Seems to work. This is with [email protected]

image

SimenB added 3 commits March 26, 2020 08:53
(101s to 24s on my machine for cached `build:ts`)
@codecov-io
Copy link

Codecov Report

Merging #9705 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9705   +/-   ##
=======================================
  Coverage   64.94%   64.94%           
=======================================
  Files         288      288           
  Lines       12184    12184           
  Branches     3020     3022    +2     
=======================================
  Hits         7913     7913           
  Misses       3634     3634           
  Partials      637      637           

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 9f0339c...85f9da1. Read the comment docs.

@SimenB SimenB merged commit d4057ce into jestjs:master Mar 26, 2020
@SimenB SimenB deleted the downlevel-ts branch March 26, 2020 08:56
Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

That's gonna add a lot of extra code to download through npm :<

@SimenB
Copy link
Member Author

SimenB commented Mar 26, 2020

the definition files aren't that large. But yes, it'll make the package bigger

@SimenB
Copy link
Member Author

SimenB commented Mar 26, 2020

@thymikee we could do something to replace the 3.8 definitions with 3.4 compatible ones?

@thymikee
Copy link
Collaborator

If that doesn't degrade DX then I'm all in 👍

@SimenB
Copy link
Member Author

SimenB commented Mar 26, 2020

It'll degrade DX for consumers, but not for us (as we'll still work on the source). See caveats (semantics): https://github.com/sandersn/downlevel-dts/blob/master/README.md
No big ones though, so possibly worth it?

@thymikee
Copy link
Collaborator

Worth it IMO

@SimenB
Copy link
Member Author

SimenB commented Mar 26, 2020

PR welcome 😀 Last caveat is that we have to ditch declarationMap, but that's broken anyways since we don't publish source files.

(I personally don't care about install size, so I won't be working it.)

@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 May 11, 2021
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.

Import issues in latest version (25.2.0)
4 participants