Prepack: Make "Model Invariant Violation" more useful

[cblappert]

When Prepack operates on a bundle and emits invariants checking whether the model is correct, we should be able to provide a more descriptive warning that describes which input file to change (the one we think is the model) as well as a suggestion of what to insert to fix the model and where.

Link to where we emit invariants: https://github.com/facebook/prepack/blob/a0ece2f8b418211254b1b4e52ade072c037a7799/src/utils/generator.js#L224

e.g.

"Prepack model invariant violation property of object in expected to be found
Suggestion: Change to "

Or in the case where the value was missing from the model, give an example line to add to the model.

[baptistemanson]

I will give it a try.

[baptistemanson]

Here is the point I reached.
With the input:

(function () {
  __assumeDataProperty(global, "unknownProperty", 1);
  })();

prepack outputs:

(function () {
  var _$0 = this;

  var _0 = _$0;

  if (_0.unknownProperty !== 1) {
    throw new Error("Prepack model invariant violation in ../notepad/test.js at (3:50): global.unknownProperty was expected to be equal to 1, got " + _0.unknownProperty);
  }
}).call(this);

which throws this error when executed:

Error: Prepack model invariant violation in ../notepad/test.js at (3:50): global.unknownProperty was expected to be equal to 1, got undefined

I may have missed test cases: I only understood how to get this error via the __assumeDataProperty function. From what I can infer, we use it when we want to inform prepack that the context has some objects + properties already set (like the window object, require, it, should? not sure). Is there any other way to trigger the "Model Invariant Violation"?

[baptistemanson]

Right now we emit Invariant with:

emitInvariant(
    args: Array<Value>,
    violationConditionFn: (Array<BabelNodeExpression>) => BabelNodeExpression,
    appendLastToInvariantFn?: BabelNodeExpression => BabelNodeExpression
  )

_emitInvariant_ doesn't know what is tested (injected violationConditionFn). It cannot throw helpful messages.
It seems there is only 3 different violationConditionFn: (typeof, !==, ===).
I can either:

// specialize each invariant
emitTypeofInvariant(node, type);
emitEqualInvariant(node, valueNode);
emitNotEqualInvariant(node, valueNode);

// or delegate the help message as well
emitInvariant(
    args: Array<Value>,
    violationConditionHelpMessage: string,
    violationConditionFn: (Array<BabelNodeExpression>) => BabelNodeExpression,
    appendLastToInvariantFn?: BabelNodeExpression => BabelNodeExpression
)

Your thoughts @cblappert ?

[NTillmann]

I like your new error text! And specialized emit-invariant functions seem like a great idea.

[DeividasK]

@NTillmann Hey Nikolai,

I already found a similar function signature to what was discussed in this issue and am not sure of whether this has been implemented or not. I couldn't find any relevant PRs after a short search. Let me know if this is something that still needs to be worked on or whether this should have been closed.

Thanks!

[NTillmann]

The invariant text is now a bit more informative, but it doesn't include the file/line number yet, that still needs to be done. I don't think we need special invariant methods anymore.

[baptistemanson]

Sorry everybody for not being super responsive! I implemented a first version of it, let me rebase and clean up.