Fix mask() working incorrectly with union() when an alternative object contains extra unknown props

[dimikot]

Fixes https://github.com/ianstormtaylor/superstruct/issues/803

Basically, it's a more proper rework of my previous PR https://github.com/ianstormtaylor/superstruct/pull/629 merged several year ago. The fact is that "mask" approach applies to object structs only, so it's more proper even conceptually to put the masking logic into object() constructor rather than having it in run(). By doing so, I also fix a bug where union() did not work properly with mask(): before, instead of removing the unknown props when a union element matches, it mistakenly threw a "Expected the value to satisfy a union" error.

Test plan

npm run test

Before, the added test threw:

After, all the tests (including the new ones) pass.

[dimikot]

@ianstormtaylor could you please take a look? This is a pretty straighforward change.

[dimikot]

@ianstormtaylor :pray:

[dimikot]

@ianstormtaylor 🙏🙏

[arturmuller]

Hey @dimikot — I have recently spoken to Ian and I will be helping out with the maintenance of Superstruct from now on.

Getting union to work with mask has actually been on the top of my priority list (it's so useful and a real bummer that it currently doesn't work!) so I am psyched that you've already taken a crack at it! ✌️

I will look into your PR closely next week and get back to you.

[arturmuller]

Ok, looked through your PR: One thing I am not sure about is whether it a problem that the output of mask with opaque object has changed if an array is passed as value.

// Before
mask([0, 1, 2], object()) // Returns { '0': 0, '1': 1, '2': 2 }

// Your PR
mask([0, 1, 2], object()) // Returns [ 0, 1, 2 ]

Admittedly, it doesn't feel like a very common use-case, and I actually think the new behaviour is more correct, but I would hate to have some patch-version-bump breakages for people in my first couple of releases.

[dimikot]

I think I broke it in the old PR https://github.com/ianstormtaylor/superstruct/pull/629 actually (this is likely where { '0': 0, '1': 1, '2': 2 } behavior was introduced first), so in the current PR, it's returning back to the roots. But I hear you, I'll try to figure out, how to keep the current behavior, even though it's a little unexpected.

[arturmuller]

Hi @dimikot — please lmk if you would like to finish this PR. 🙏

[arturmuller]

@dimikot — thank you for your work on this. I've merged it into main in a separate PR as I wasn't able to make changes in your branch, but your commit (and most importantly the fix itself) have made it into 2.0! Thank you so much 🙌