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

TS 4.9 compat #587

Merged
merged 5 commits into from Nov 4, 2022
Merged

TS 4.9 compat #587

merged 5 commits into from Nov 4, 2022

Conversation

markerikson
Copy link
Contributor

@markerikson markerikson commented Nov 3, 2022

Pasting the main comment from the big commit:

Per microsoft/TypeScript#50831 , the existing implementation of MergeParameters broke with TS 49 due to a change in how "optional" and "undefined" arguments get interpreted

Anders Hjelsberg himself supplied a new implementation that's actually much simpler, but only works with TS 4.7 and greater.

Normally this would require shipping two entirely different sets of
types, as we already do for TS <4.1. However, there's a trick we used
with RTK where we:

  • Create a folder and put two different files with different impls of the same type inside
  • Add an index file and re-export one of those
  • Add a file named package.dist.json containing typesVersions that points to both of those .d.ts files
  • Copy that package.dist.json over to dist/some/package.json during the actual build/publish step

That way during dev TS just imports the type as normal, but the built
version sees that some/package.json, sees typesVersions, finds the
right .d.ts file, and imports the right implementation for itself.

@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Merging #587 (474b12c) into master (ae67c1f) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 474b12c differs from pull request most recent head 11ed107. Consider uploading reports for the commit 11ed107 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #587   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          105       105           
  Branches        24        24           
=========================================
  Hits           105       105           
Impacted Files Coverage Δ
src/index.ts 100.00% <100.00%> (ø)

@markerikson markerikson force-pushed the feature/ts-4.9-compat branch 3 times, most recently from 17735af to 456a04a Compare November 3, 2022 04:35
Per microsoft/TypeScript#50831 , the existing
implementation of `MergeParameters` broke with TS 49 due to a change
in how "optional" and "undefined" arguments get interpreted

Anders Hjelsberg himself supplied a new implementation that's actually
much simpler, but only works with TS 4.7 and greater.

Normally this would require shipping two entirely different sets of
types, as we already do for TS <4.1. However, there's a trick we used
with RTK where we:

- Create a folder and put two different files with different impls of
  the same type inside
- Add an index file and re-export one of those
- Add a file named `package.dist.json` containing `typesVersions` that
  points to both of those `.d.ts` files
- Copy that `package.dist.json` over to `dist/some/package.json` during
  the actual build/publish step

That way during dev TS just imports the type as normal, but the built
version sees that `some/package.json`, sees `typesVersions`, finds the
right `.d.ts` file, and imports the right implementation for itself.
@markerikson
Copy link
Contributor Author

Confirmed this works by:

  • Creating a new Vite React+TS app project
  • Copying the entire typetest file into that
  • doing a yalc publish from this branch and installing into that repo
  • taking turns installing TS 4.2, 4.6, 4.8, and 4.9-rc and running yarn tsc to confirm that the typetest file compiles cleanly
  • uncommenting the dead chunk around like 615 where TS 4.8 and earlier had a // @ts-expect-error, and confirming that 4.9 panics because there isn't an error (also confirming that they all really were compiling the file)

@markerikson markerikson merged commit e4c2ace into master Nov 4, 2022
@markerikson markerikson deleted the feature/ts-4.9-compat branch November 4, 2022 01:41
silasbw pushed a commit to valora-inc/wallet that referenced this pull request Jan 10, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [reselect](https://togithub.com/reduxjs/reselect) | [`^4.0.0` ->
`^4.1.7`](https://renovatebot.com/diffs/npm/reselect/4.1.6/4.1.7) |
[![age](https://badges.renovateapi.com/packages/npm/reselect/4.1.7/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/reselect/4.1.7/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/reselect/4.1.7/compatibility-slim/4.1.6)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/reselect/4.1.7/confidence-slim/4.1.6)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>reduxjs/reselect</summary>

###
[`v4.1.7`](https://togithub.com/reduxjs/reselect/releases/tag/v4.1.7)

[Compare
Source](https://togithub.com/reduxjs/reselect/compare/v4.1.6...v4.1.7)

This release updates the TS types to work correctly with TS 4.9, which
made a change that broke the existing `MergeParameters` type
implementation. Happily, the TS team [provided a better (and simpler!)
`MergeParameters`
implementation](https://togithub.com/microsoft/TypeScript/pull/50831#issuecomment-1253830522).
Since that only works with TS 4.7+, we've reworked the internals to
handle providing the old implementation to TS 4.2..4.6, and the new
implementation to TS 4.7 and greater.

As a user, there should be no visible change - just update to 4.1.7.

#### What's Changed

- Include 4.6 in the TS test matrix by
[@&#8203;lukeapage](https://togithub.com/lukeapage) in
[reduxjs/reselect#576
- TS 4.9 compat by
[@&#8203;markerikson](https://togithub.com/markerikson) in
[reduxjs/reselect#587

**Full Changelog**:
reduxjs/reselect@v4.1.6...v4.1.7

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/valora-inc/wallet).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC45NS4wIiwidXBkYXRlZEluVmVyIjoiMzQuOTUuMCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
kathaypacific pushed a commit to valora-inc/wallet that referenced this pull request Jan 10, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [reselect](https://togithub.com/reduxjs/reselect) | [`^4.0.0` ->
`^4.1.7`](https://renovatebot.com/diffs/npm/reselect/4.1.6/4.1.7) |
[![age](https://badges.renovateapi.com/packages/npm/reselect/4.1.7/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/reselect/4.1.7/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/reselect/4.1.7/compatibility-slim/4.1.6)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/reselect/4.1.7/confidence-slim/4.1.6)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>reduxjs/reselect</summary>

###
[`v4.1.7`](https://togithub.com/reduxjs/reselect/releases/tag/v4.1.7)

[Compare
Source](https://togithub.com/reduxjs/reselect/compare/v4.1.6...v4.1.7)

This release updates the TS types to work correctly with TS 4.9, which
made a change that broke the existing `MergeParameters` type
implementation. Happily, the TS team [provided a better (and simpler!)
`MergeParameters`
implementation](https://togithub.com/microsoft/TypeScript/pull/50831#issuecomment-1253830522).
Since that only works with TS 4.7+, we've reworked the internals to
handle providing the old implementation to TS 4.2..4.6, and the new
implementation to TS 4.7 and greater.

As a user, there should be no visible change - just update to 4.1.7.

#### What's Changed

- Include 4.6 in the TS test matrix by
[@&#8203;lukeapage](https://togithub.com/lukeapage) in
[reduxjs/reselect#576
- TS 4.9 compat by
[@&#8203;markerikson](https://togithub.com/markerikson) in
[reduxjs/reselect#587

**Full Changelog**:
reduxjs/reselect@v4.1.6...v4.1.7

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/valora-inc/wallet).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC45NS4wIiwidXBkYXRlZEluVmVyIjoiMzQuOTUuMCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

None yet

1 participant