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

Add Mutable type #157

Merged
merged 29 commits into from Dec 31, 2020
Merged

Add Mutable type #157

merged 29 commits into from Dec 31, 2020

Conversation

kainiedziela
Copy link
Contributor

@kainiedziela kainiedziela commented Nov 26, 2020

Fixes #146

SetMutable

Creates a type that converts the given keys from readonly to mutable. The remaining keys are kept as is.

Use-case: You want to define a single model where the only thing that changes is whether or not some of the keys are mutable.

import {SetMutable} from 'type-fest';

type Foo = {
	readonly a: number;
	b: string;
	readonly c: boolean;
}

type SomeMutable = SetMutable<Foo, 'b' | 'c'>;
// type SomeMutable = {
// 	readonly a: number;
// 	b: string; // Was already mutable and still is.
// 	c: boolean; // Is now mutable.
// }

source/set-mutable.d.ts Outdated Show resolved Hide resolved
source/set-mutable.d.ts Outdated Show resolved Hide resolved
source/set-mutable.d.ts Outdated Show resolved Hide resolved
source/set-mutable.d.ts Outdated Show resolved Hide resolved
@papb
Copy link
Contributor

papb commented Nov 28, 2020

Hello, nice work, is there a plan to create SetReadonly as well?

@sindresorhus
Copy link
Owner

Hello, nice work, is there a plan to create SetReadonly as well?

Sure. Open an issue or PR about it.

Copy link
Collaborator

@voxpelli voxpelli left a comment

Choose a reason for hiding this comment

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

Sorry for not reviewing earlier, I had my notifications for this repo setup in a way that made me miss the request for reviews.

My "request changes" isn't for all of my comments, but mostly for things like the erroneous comments in the test file.

As a new co-maintainer on this repo I'm still figuring out some things myself.

source/set-mutable.d.ts Outdated Show resolved Hide resolved
source/set-mutable.d.ts Outdated Show resolved Hide resolved
source/set-mutable.d.ts Outdated Show resolved Hide resolved
test-d/set-mutable.ts Outdated Show resolved Hide resolved
source/mutable.d.ts Outdated Show resolved Hide resolved
kainiedziela and others added 2 commits December 1, 2020 13:59
Co-authored-by: Pelle Wessman <pelle@kodfabrik.se>
Copy link
Collaborator

@voxpelli voxpelli left a comment

Choose a reason for hiding this comment

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

Wow, you're fast, I only had time to review #153 and then I'm back to this one 😅

Well, here's some follow up thoughts

source/mutable.d.ts Outdated Show resolved Hide resolved
source/mutable.d.ts Outdated Show resolved Hide resolved
source/set-mutable.d.ts Outdated Show resolved Hide resolved
kainiedziela and others added 4 commits December 1, 2020 15:11
Co-authored-by: Pelle Wessman <pelle@kodfabrik.se>
Co-authored-by: Pelle Wessman <pelle@kodfabrik.se>
test-d/set-mutable.ts Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
source/mutable.d.ts Outdated Show resolved Hide resolved
source/simplify.d.ts Outdated Show resolved Hide resolved
kainiedziela and others added 4 commits December 17, 2020 22:58
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
@papb
Copy link
Contributor

papb commented Dec 18, 2020

What do you think of also making Mutable<readonly [string, number] return [string, number]? I needed this recently and had to write my own.

readme.md Outdated Show resolved Hide resolved
@kainiedziela
Copy link
Contributor Author

What do you think of also making Mutable<readonly [string, number] return [string, number]? I needed this recently and had to write my own.

This sounds useful to me. @sindresorhus thoughts?

@voxpelli
Copy link
Collaborator

I think we should open a separate issue / PR for accepting arrays into Mutable, to keep this PR as focused as possible so we can get it reviewed and merged without too much effort 😉

Sorry for lagging with my re-review, I’m a bit behind at work right now.

@sindresorhus
Copy link
Owner

I think we should open a separate issue / PR for accepting arrays into Mutable, to keep this PR as focused as possible so we can get it reviewed and merged without too much effort 😉

Agreed: #176

@sindresorhus
Copy link
Owner

@voxpelli LGTY?

@sindresorhus
Copy link
Owner

Actually, I missed that the tests are failing. You need to fix those.

Copy link
Collaborator

@voxpelli voxpelli left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@voxpelli
Copy link
Collaborator

@sindresorhus Do you want me to merge?

@sindresorhus
Copy link
Owner

Sure, go ahead.

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.

[Proposal] Writable and SetWritable
5 participants