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.

Sign up for GitHub

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

Added transform function (instead of update) #23

Merged
merged 3 commits into from
Apr 9, 2018
Merged

Added transform function (instead of update) #23

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d82676
Added update function
twop Apr 8, 2018
a0c0354
improved typings for tag prop in config obj
twop Apr 8, 2018
115177e
replaced update with transform for immutable updates/state transitions
twop Apr 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,3 +279,48 @@ describe('config object', () => {
expect(A.a(1)).toEqual({ tag: 'a', val: 1 });
});
});

describe('update suite', () => {
const Paylod = unionize(
{
num: ofType<number>(),
str: ofType<string>(),
},
{ value: 'payload' },
);

it('skips unmet cases', () => {
const num = Paylod.num(1);
expect(Paylod.update(num, { str: s => s + '?' })).toBe(num);
expect(Paylod.update({ str: s => s + '??' })(num)).toBe(num);
});

it('updates with met cases', () => {
const str = Paylod.str('hi');
const exclaim = (s: string) => s + '!';
const exclaimed = Paylod.str('hi!');
expect(Paylod.update({ str: exclaim })(str)).toEqual(exclaimed);
expect(Paylod.update(str, { str: exclaim })).toEqual(exclaimed);
});

it('updates immutably by partial state', () => {
const Data = unionize({
A: ofType<{ a: 'not used' }>(),
BC: ofType<{ b: number; c: string }>(),
});

const bc = Object.freeze(Data.BC({ b: 1, c: 'hi' }));
const expected = Data.BC({ b: 1, c: 'hi!' });
expect(Data.update({ BC: ({ c }) => ({ c: c + '!' }) })(bc)).toEqual(expected);
expect(Data.update(bc, { BC: ({ c }) => ({ c: c + '!' }) })).toEqual(expected);
});

it('still updated even with value prop', () => {
const Data = unionize({ BC: ofType<{ b: number; c: string }>() }, { value: 'payload' });

const bc = Object.freeze(Data.BC({ b: 1, c: 'hi' }));
const expected = Data.BC({ b: 1, c: 'hi!' });
expect(Data.update({ BC: ({ c }) => ({ c: c + '!' }) })(bc)).toEqual(expected);
expect(Data.update(bc, { BC: ({ c }) => ({ c: c + '!' }) })).toEqual(expected);
});
});
45 changes: 37 additions & 8 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export type Unionized<Record, TaggedRecord> = {
is: Predicates<TaggedRecord>;
as: Casts<Record, TaggedRecord[keyof TaggedRecord]>;
match: Match<Record, TaggedRecord[keyof TaggedRecord]>;
update: Update<Record, TaggedRecord[keyof TaggedRecord]>;
} & Creators<Record, TaggedRecord>;

export type Creators<Record, TaggedRecord> = {
Expand All @@ -30,6 +31,15 @@ export type Match<Record, Union> = {
<A>(variant: Union, cases: MatchCases<Record, Union, A>): A;
};

export type UpdateCases<Record> = Partial<
{ [T in keyof Record]: (value: Record[T]) => Partial<Record[T]> }
Copy link
Owner

Choose a reason for hiding this comment

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

Using Partial for the return type here is unsafe because it allows overwriting values with undefined. I'm not sure if inference would be able to handle using Pick though... Perhaps we should statically disallow values to be undefined, which we're already doing in the case of SingleValueVariants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is possible to not use Partial + remove this magic check for 'isObject'.
I think the way to do it is this:

function update<T>(val:T):T | Part<T> {}; // Not Partial
type Part<T> = {_opaque:T}//  is opaque for user.

// helper function to create a part
function part<K extends keyof T, T>(p: Pick<K,T>) : Part<T> {
   // tag that this is a part object for implementation
   return  {__part: p} as any as Part<T>
}

// simplified implementation for update
function updateObject(oldV, newV) {
if (typeof newV === 'object'  && newV.__part)  {
   // merge objects
   return merged.
   }
   // if it is not a part it has to be a full object.
   return newV;
}

So you either have to provide a complete object (not Partial) or use a helper function.

const Data = unionize({AB: ofType<{ a: number; b: string }>()});
const ab = Data.AB({ a: 1, b: 'hi' });
Data.update(ab, { AB: ({ b }) => part({ b: b + '!' }) }); //note usage of 'part'
Data.update(ab, { AB: ({ a, b }) => ({a, b: b + '!' }) }); // or provide a full shape

Not 100% that it is worth complexity. But thinking about updates again, I would either prefer a solution with 'part' function without 'isObject' magic or forbid partial updates. Note that in that case it is still useful:

  • don't have to provide default
  • don't have to call type constructor
// a bit verbose
Data.match(ab, { AB: ({ a, b }) => Data.AB({a, b: b + '!' }), default: i=>i })

>;

export type Update<Record, Union> = {
(cases: UpdateCases<Record>): (variant: Union) => Union;
(variant: Union, cases: UpdateCases<Record>): Union;
};

export type MultiValueVariants<Record extends MultiValueRec, TagProp extends string> = {
[T in keyof Record]: { [_ in TagProp]: T } & Record[T]
};
Expand All @@ -55,26 +65,29 @@ export type NoDefaultRec<Val> = {
*
* @param record A record mapping tags to value types. The actual values of the record don't
* matter; they're just used in the types of the resulting tagged union. See `ofType`.
* @param tagProp An optional custom name for the tag property of the union.
* @param valProp An optional custom name for the value property of the union. If not specified,
* @param config An optional config object. By default tag='tag' and payload is merged into object itself
Copy link
Owner

Choose a reason for hiding this comment

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

For consistency, let's say "value" instead of "payload" here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

* @param config.tag An optional custom name for the tag property of the union.
* @param config.value An optional custom name for the value property of the union. If not specified,
* the value must be a dictionary type.
*/

export function unionize<
Record extends SingleValueRec,
TagProp extends string,
ValProp extends string
ValProp extends string,
TagProp extends string = 'tag'
>(
record: Record,
config: { value: ValProp; tag?: TagProp },
): Unionized<Record, SingleValueVariants<Record, TagProp, ValProp>>;
export function unionize<Record extends MultiValueRec, TagProp extends string>(
export function unionize<Record extends MultiValueRec, TagProp extends string = 'tag'>(
record: Record,
config?: { tag: TagProp },
): Unionized<Record, MultiValueVariants<Record, TagProp>>;
export function unionize<Record>(record: Record, config?: { value?: string; tag?: string }) {
const { value: valProp = undefined, tag: tagProp = 'tag' } = config || {};

const payload = (variant: any) => (valProp ? variant[valProp] : variant);
Copy link
Owner

Choose a reason for hiding this comment

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

s/payload/value/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted a helper function that will extract value from an object. What about 'extractVal()'?


const creators = {} as Creators<Record, any>;
for (const tag in record) {
creators[tag] = (value: any) =>
Expand All @@ -89,9 +102,7 @@ export function unionize<Record>(record: Record, config?: { value?: string; tag?
function evalMatch(variant: any, cases: any): any {
const k = variant[tagProp];
const handler = cases[k];
return handler !== undefined
? handler(valProp ? variant[valProp] : variant)
: cases.default(variant);
return handler !== undefined ? handler(payload(variant)) : cases.default(variant);
}

const match = (first: any, second?: any) =>
Expand All @@ -107,17 +118,35 @@ export function unionize<Record>(record: Record, config?: { value?: string; tag?
});
}

const evalUpd = (variant: any, cases: any): any => {
const k: keyof Record = variant[tagProp];
return k in cases
? creators[k](immutableUpd(payload(variant), cases[k](payload(variant))))
: variant;
};

const update = (first: any, second?: any) =>
second ? evalUpd(first, second) : (variant: any) => evalUpd(variant, first);

return Object.assign(
{
is,
as,
match,
update,
_Record: record,
},
creators,
);
}

// Should we merge objects or just replace them?
// was unable to find a better solution to that
const objType = Object.prototype.toString.call({});
const isObject = (maybeObj: any) => Object.prototype.toString.call(maybeObj) === objType;
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, I'm not sure how reliable or efficient this sort of check is. Does it even make sense to "update" something that's a primitive type? If not, could we statically forbid using this method in that case?

Copy link
Contributor Author

@twop twop Apr 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think it does make sense to have update for primitive types (and things like Date, Array etc).
string -> string, number->number, array->array. So the output has to match exactly the input type.

But for records (plain object) it makes sense to have partial update. The best reference that I found is react setState signature. They use Pick<K,T> type and not partial.

const immutableUpd = (old: any, updated: any) =>
isObject(old) ? Object.assign({}, old, updated) : updated;

/**
* Creates a pseudo-witness of a given type. That is, it pretends to return a value of
* type `T` for any `T`, but it's really just returning `undefined`. This white lie
Expand Down