loathers / grimoire

Apache License 2.0
3 stars 8 forks source link

outfit.spec() behaves poorly with Outfit.from() #79

Closed horrible-little-slime closed 1 year ago

horrible-little-slime commented 1 year ago

I assume that this relates to an extremely annoying aspect of JavaScript and also TypeScript (typescript has a separate, extra-annoying part where it doesn't resolve it);

if I have a type type ExampleType = { foo?: string }, two very different objects could fulfill it: { foo: undefined }, and {} both fit that type because foo is an optional parameter. However, when we put an ExampleType into an object spreader, bad things happen:

const DEFAULT = { foo: "bar" };
const test = {};
return { ...DEFAULT, ...test}; // { foo: "bar" }
const DEFAULT = { foo: "bar" };
const test = { foo: undefined };
return {...DEFAULT, ...test} // {foo: undefined}

Because outfit.spec() populates the spec with a bunch of undefineds instead of leaving the entries truly blank, we can get some ugly behavior when using object spread syntax with it.

My attitude is that we should only add things to the spec object if they are truthy, and I'll put in a PR as such, but this is a sufficiently spicy issue that I'm making a github Issue for it beforehand, lest we need to modify things.

The moral of the story is some freecandy code went from looking like combatOutfit(outfit.spec()) to

combatOutfit(
  Object.fromEntries(Object.entries(outfit.spec()).filter(([, value]) => value))
);