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

chore(deps): update dependency expect-type to v0.13.0 #14032

Merged
merged 1 commit into from Apr 13, 2022

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Jan 31, 2022

WhiteSource Renovate

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
expect-type 0.12.0 -> 0.13.0 age adoption passing confidence

Release Notes

mmkal/ts

v0.13.0

minor changes

  • Fix constructor parameter comparison (#​245)

Configuration

πŸ“… Schedule: 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.


  • If you want to rebase/retry this PR, click this checkbox.

This PR has been generated by WhiteSource Renovate. View repository job log here.

@WikiRik
Copy link
Member

WikiRik commented Jan 31, 2022

@ephys could you maybe take a look at this in the coming week or so?

@ephys ephys self-assigned this Jan 31, 2022
@ephys
Copy link
Member

ephys commented Feb 6, 2022

I've tried but I can't manage to make the typing tests for the mutability of hooks options work in TS [4.2,4.4] :/

@WikiRik
Copy link
Member

WikiRik commented Feb 6, 2022

Do you think it's a bug with expect-type or with our typings?

@ephys
Copy link
Member

ephys commented Feb 6, 2022

I managed to narrow the issue down to NoReadonlyArraysDeep.
The error was also present in my temporary test that did not use expect-type, so it's more likely that expect-type wasn't reporting an existing issue before.

Since the test passes starting with TS 4.5, I think our code might be correct. It might be a limitation of TS < 4.5. Not sure why it works in TS 4.1.

I'm pretty sure TS < 4.5 does not like us mapping the entries of an array.

Looked into whether other libraries had implemented this, such as type-fest but did not find anything.

I was still sick when I looked into this, we could look for more implementations of a DeepMutable type and see if it's done differently.

@ephys
Copy link
Member

ephys commented Feb 11, 2022

I think I figured it out.

The test was making sure the whole options bag was mutable

The option bags that were failing all accepted where: WhereOptions

A lot of pieces in WhereOptions accept readonly arrays

I've removed the readonly arrays as a first draft and it works.

The thing is, these readonly arrays were added for a reason:
image

But the test to ensure they're writable was also added for a reason:
image

We have two concepts at odds with eachother:

  • One wants to have the whole options bag mutable to modify it in the hooks
  • The other wants to be able to use readonly arrays.

We're going to have to displease one or the other. We won't be able to please everyone and we'll need to choose which user is going to be wrong.

Proposition : we check for mutability up to the where option. i.e. it can be replaced in hooks, but not mutated

Don't merge yet!

@WikiRik
Copy link
Member

WikiRik commented Feb 11, 2022

I think I like that proposition, shall we add that in a separate PR and then merge this update afterwards?

@ephys
Copy link
Member

ephys commented Feb 11, 2022

I'll reset this PR and do it here, so we can make sure the typing tests are passing :)

@fzn0x fzn0x added the dependency For issues and PRs. Things that are related to one or more dependencies. label Mar 11, 2022
@WikiRik
Copy link
Member

WikiRik commented Apr 13, 2022

@ephys did you fix this issue when working on the WhereOptions typing? Or is this something to keep in mind for #14020 ?

@ephys
Copy link
Member

ephys commented Apr 13, 2022

That PR didn't fix it
It's only blocking for #14091 but that PR is also blocked by #14302, so this PR hasn't been a priority for me

Let's reset this PR though

@renovate renovate bot force-pushed the renovate/expect-type-0.x branch from efe0274 to 353a841 Compare April 13, 2022 10:39
@renovate renovate bot force-pushed the renovate/expect-type-0.x branch from 353a841 to a5362d6 Compare April 13, 2022 10:40
@WikiRik
Copy link
Member

WikiRik commented Apr 13, 2022

It seems to work now in 4.4 so I guess we can merge this

@ephys
Copy link
Member

ephys commented Apr 13, 2022

That could mean I broke something in #14368
But might as well merge this and fix both the test and the issue in a follow-up PR if something arises

@WikiRik WikiRik merged commit 4979381 into main Apr 13, 2022
@WikiRik WikiRik deleted the renovate/expect-type-0.x branch April 13, 2022 10:54
@github-actions
Copy link
Contributor

πŸŽ‰ This PR is included in version 7.0.0-alpha.11 πŸŽ‰

The release is available on:

Your semantic-release bot πŸ“¦πŸš€

vanthome pushed a commit to vanthome/sequelize that referenced this pull request Jun 12, 2022
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependency For issues and PRs. Things that are related to one or more dependencies. released on @v7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants