[Feature] Provide better message when using getProp

[Shinigami92]

I have a checker and using it to check some of it props

const propertyName: string = nameof<MyModel>((o) => o.headlines);
// propertyName = 'headlines'
// value is an array / should be [string, string, string?]
MyModelChecker.getProp(propertyName).strictCheck(value);

I provoked an error and the result was

Error: value[0] is not a string
    at new VError (util.js?5394:20)
    at DetailContext.getError (util.js?5394:91)
    at Checker._doCheck (index.js?7160:179)
    at Checker.strictCheck (index.js?7160:101)
    at validator (MyComponent.ts?b499:24)
    at assertProp (vue.esm.js?a026:1722)
    at validateProp (vue.esm.js?a026:1641)
    at loop (vue.esm.js?a026:4668)
    at initProps (vue.esm.js?a026:4701)
    at initState (vue.esm.js?a026:4642)

Could we provide the parameter value of getProp in the message, so I could see which property is checked?

The message could be headlines[0] is not a string
or it could be MyModelChecker's headlines found: value[0] is not a string
or similar

I'm open to discussion and I'm willing to implement this feature once we've found a good design

[Shinigami92]

So before I implemented your package, my solution locked like this:

@Prop({
  required: true,
  validator(value: any): boolean {
    const propertyName: string = nameof<MyModel>((o) => o.headlines);
    if (!value) {
      console.warn(`The property '${propertyName}' should be defined, found: ${typeof value}`);
      return false;
    }
    if (!Array.isArray(value)) {
      console.warn(`The property '${propertyName}' is not an array, found: ${typeof value}`);
      return false;
    }
    if (value.length < 2) {
      console.warn(
        `The property '${propertyName}' may contain a minimum of two elements, found: ${value.length}`
      );
      return false;
    }
    if (value.length > 3) {
      console.warn(
        `The property '${propertyName}' may contain a maximum of three elements, found: ${value.length}`
      );
      return false;
    }
    if (typeof value[0] !== 'string') {
      console.warn(
        `The index 0 of property '${propertyName}' should be of type string, found: ${typeof value[0]}`
      );
      return false;
    }
    if (typeof value[1] !== 'string') {
      console.warn(
        `The index 1 of property '${propertyName}' should be of type string, found: ${typeof value[1]}`
      );
      return false;
    }
    if (!(!value[2] || typeof value[2] === 'string')) {
      console.warn(
        `The index 2 of property '${propertyName}' should be undefined or of type string,` +
          ` found: ${typeof value[2]}`
      );
      return false;
    }
    return true;
  }
})
public readonly headlines!: [string, string, string?];

And now:

const MyModelChecker: Checker = createCheckers(MyModelTI).MyModel;
// ...
@Prop({
  required: true,
  validator(value: any): boolean {
    const propertyName: string = nameof<MyModel>((o) => o.headlines);
    MyModelChecker.getProp(propertyName).strictCheck(value);
    return true;
  }
})
public readonly headlines!: [string, string, string?];

Beside nameof is a very nice package.

[dsagal]

This is not unreasonable, in fact, including a diff below which might be what you are after. On the other hand, if the caller is using getProp, then the property is name available, so perhaps it's OK (better?) to leave it to the caller to format errors appropriately.

E.g. in your case, you could add a helper:

function getCheckerFunc(rootChecker: Checker, propertyName: string) {
  const checker = rootChecker.getProp(propertyName);
  return (value: any) => {
    try {
      checker.strictCheck(value);
    } catch (e) {
      e.message = `Property '${propertyName}': ${e.message}`;
      throw e;
    }
  };
}

Diff of possible implementation changing getProp() to throw errors of the form propName is not a ...:

diff --git a/lib/index.ts b/lib/index.ts
index 46490b3..b59d106 100644
--- a/lib/index.ts
+++ b/lib/index.ts
@@ -43,7 +43,7 @@ export class Checker {
   private checkerStrict: CheckerFunc;

   // Create checkers by using `createCheckers()` function.
-  constructor(private suite: ITypeSuite, private ttype: TType) {
+  constructor(private suite: ITypeSuite, private ttype: TType, private _path?: string) {
     if (ttype instanceof TIface) {
       for (const p of ttype.props) {
         this.props.set(p.name, p.ttype);
@@ -104,7 +104,7 @@ export class Checker {
   public getProp(prop: string): Checker {
     const ttype = this.props.get(prop);
     if (!ttype) { throw new Error(`Type has no property ${prop}`); }
-    return new Checker(this.suite, ttype);
+    return new Checker(this.suite, ttype, prop);
   }

   /**
@@ -160,7 +160,7 @@ export class Checker {
     if (!checkerFunc(value, noopCtx)) {
       const detailCtx = new DetailContext();
       checkerFunc(value, detailCtx);
-      throw detailCtx.getError();
+      throw detailCtx.getError(this._path);
     }
   }

@@ -171,7 +171,7 @@ export class Checker {
     }
     const detailCtx = new DetailContext();
     checkerFunc(value, detailCtx);
-    return detailCtx.getErrorDetail();
+    return detailCtx.getErrorDetail(this._path);
   }

   private _getMethod(methodName: string): TFunc {
diff --git a/lib/util.ts b/lib/util.ts
index d909f93..cab6a83 100644
--- a/lib/util.ts
+++ b/lib/util.ts
@@ -83,8 +83,7 @@ export class DetailContext implements IContext {
     }
   }

-  public getError(): VError {
-    let path: string = "value";
+  public getError(path: string = "value"): VError {
     const msgParts: string[] = [];
     for (let i = this._propNames.length - 1; i >= 0; i--) {
       const p = this._propNames[i];
@@ -97,8 +96,7 @@ export class DetailContext implements IContext {
     return new VError(path, msgParts.join("; "));
   }

-  public getErrorDetail(): IErrorDetail|null {
-    let path: string = "value";
+  public getErrorDetail(path: string = "value"): IErrorDetail|null {
     const details: IErrorDetail[] = [];
     for (let i = this._propNames.length - 1; i >= 0; i--) {
       const p = this._propNames[i];

[Shinigami92]

I would like to apply the diff.
I am not interested in reproducing the exact messages.
I just want to know where it comes from.

[dsagal]

Would #15 work well for you?

[Shinigami92]

Tested it with yarn add https://github.com/gristlabs/ts-interface-checker\#519c0bec4fc9dc5dda5944641e62686ccf1b4106

Current output is:

Error: value.headlines[0] is not a string
    at new VError (util.js?5394:20)
    at DetailContext.getError (util.js?5394:91)
    at Checker._doCheck (index.js?7160:180)
    at Checker.strictCheck (index.js?7160:102)
    at validator (MyModel.ts?b499:24)
    at assertProp (vue.esm.js?a026:1722)
    at validateProp (vue.esm.js?a026:1641)
    at loop (vue.esm.js?a026:4668)
    at initProps (vue.esm.js?a026:4701)
    at initState (vue.esm.js?a026:4642)

It's nice to see the propertyName, but the value. isn't needed (in this case) 🤷‍♂️

MyModelChecker.getProp(propertyName).strictCheck(value);

value is not myModel, it's the headlines of myModel directly not myModel.headlines
Uff that reads confusingly, hope you get it.

Nevermind, it's better than before 👍

[dsagal]

I'd like for the message to clarify that it's talking about a property, so value.headlines or just .headlines? Or would it be better to support another argument at some point, to be able to say myModel.headlines?

[Shinigami92]

I would prefer another optional argument
Or a message like Property headlines at index 0 is not a string

[dsagal]

I added setReportedPath() method. E.g.

MyModelChecker.setReportedPath('MyModel');
MyModelChecker.getProp(propertyName).strictCheck(value);

Should report, on error, MyModel.headlines[0] is not a string.
I released a patch version 0.1.9.

[Shinigami92]

working as intended