Internal: add push-slice-pop optimization

[gcanti]

@sledorze the gist is

• context is mutable (I feel a bit dirty for this) and I push / pop ContextEntrys while traversing the tree
• if an error is encountered then I take a snapshot of the current context by calling context.slice() in the failure API

Analysis:

In the best case (which we should optimize for) only one array is created and no snapshots at all.
In the worst case (all leafs are invalid) I guess it does less work as well

Benchmarks:

Baseline (from last PR)

space-object (good) x 666,376 ops/sec ±0.40% (88 runs sampled)
space-object (bad) x 587,146 ops/sec ±0.90% (85 runs sampled)

after this change

space-object (good) x 1,021,748 ops/sec ±0.39% (89 runs sampled)
space-object (bad) x 748,862 ops/sec ±0.64% (89 runs sampled)

The main point is: this change looks backward compatible if the failure API is consistently used, or am I wrong?

[sledorze]

@gcanti I think it's a breaking change as the reporting will immediately break for end user that have implemented custom Types with the validate API (public).

---

Also, if a 'no breaking change' is no more a constraint; what would we choose?

From my subjective perspective we have two solutions that look similar in speed gain but:

• One is mutable (brittle) and the user should know it.
• The other has a different report structure, (which I see as a feature - having more information; the hierarchy) but is a little more subtil in the parameters to pass to the validate function (the additional key).

I'm wondering if we should not try decide first of a solution (with enhancement or maybe another alternative) before implementing one in a hurry and change it at the next breaking change (soon).

---

I am saying that as 2.8 is around the corner and Yes, I would love to have a nicer solution than intersecting with partials to have optional members.. :)

[gcanti]

I think it's a breaking change as the reporting will immediately break for end user that have implemented custom Types with the validate API

You mean if the end user makes use of something else to create a failure other than the failure API right?

const MyString = new t.Type<string>(
  'string',
  (m): m is string => typeof m === 'string',
  // refuse to use the `failure` API here
  (m, c) => (typeof m === 'string' ? t.success(m) : t.failures([t.getValidationError(m, c)])),
  t.identity
)

const Person = t.type(
  {
    name: MyString,
    age: t.number
  },
  'Person'
)

console.log(PathReporter.report(Person.decode({})))
/*
[ 'Invalid value undefined supplied to : Person', // <= wrong error message here
  'Invalid value undefined supplied to : Person/age: number' ]
*/

That's my main concern too, if the failure API is always used then you won't notice a different behaviour.

However I can't be sure of that in user land, so this change would be too dangerous.

Closing this, thanks for reviewing (and for the great work you did in the last few days on this subject).

[gcanti]

@sledorze as a side note, I noticed that the context is typed using Array (type Context = Array<ContextEntry>) instead of ReadonlyArray. Should we fix that as a bug?

[sledorze]

@gcanti ah.. that's a good question; it may break 'working' but wrong code, so I think yes, we should release it as a bug fix.