[8.6] [@kbn/handlebars] Ensure only decorators have an options.args…

… property (elastic#147791) (elastic#147799)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[@kbn/handlebars] Ensure only decorators have an `options.args`
property (elastic#147791)](elastic#147791)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Thomas
Watson","email":"watson@elastic.co"},"sourceCommit":{"committedDate":"2022-12-19T19:40:18Z","message":"[@kbn/handlebars]
Ensure only decorators have an `options.args` property
(elastic#147791)\n\nAfter adding support for decorators an `args` property has
added to\r\n`HelperOptions` (which is shared with decorators). To not
leak this into\r\nregular helpers and to align with the upstream
handlebars, this PR\r\nremoves the `args` property from regular helpers
so that it's only\r\nvisible to
decorators.","sha":"800f45180cb1706a67f264ba5cee95c1a2031b8a","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:prev-minor","v8.7.0"],"number":147791,"url":"elastic#147791
Ensure only decorators have an `options.args` property
(elastic#147791)\n\nAfter adding support for decorators an `args` property has
added to\r\n`HelperOptions` (which is shared with decorators). To not
leak this into\r\nregular helpers and to align with the upstream
handlebars, this PR\r\nremoves the `args` property from regular helpers
so that it's only\r\nvisible to
decorators.","sha":"800f45180cb1706a67f264ba5cee95c1a2031b8a"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"elastic#147791
Ensure only decorators have an `options.args` property
(elastic#147791)\n\nAfter adding support for decorators an `args` property has
added to\r\n`HelperOptions` (which is shared with decorators). To not
leak this into\r\nregular helpers and to align with the upstream
handlebars, this PR\r\nremoves the `args` property from regular helpers
so that it's only\r\nvisible to
decorators.","sha":"800f45180cb1706a67f264ba5cee95c1a2031b8a"}}]}]
BACKPORT-->

Co-authored-by: Thomas Watson <watson@elastic.co>

packages/kbn-handlebars/index.test.ts

@@ -49,51 +49,118 @@ Expecting 'ID', 'STRING', 'NUMBER', 'BOOLEAN', 'UNDEFINED', 'NULL', 'DATA', got
   }
 });
 
-it('Only provide options.fn/inverse to block helpers', () => {
-  function toHaveProperties(...args: any[]) {
-    toHaveProperties.calls++;
-    const options = args[args.length - 1];
-    expect(options).toHaveProperty('fn');
-    expect(options).toHaveProperty('inverse');
-    return 42;
-  }
-  toHaveProperties.calls = 0;
-
-  function toNotHaveProperties(...args: any[]) {
-    toNotHaveProperties.calls++;
-    const options = args[args.length - 1];
-    expect(options).not.toHaveProperty('fn');
-    expect(options).not.toHaveProperty('inverse');
-    return 42;
-  }
-  toNotHaveProperties.calls = 0;
+// Extra "helpers" tests
+describe('helpers', () => {
+  it('Only provide options.fn/inverse to block helpers', () => {
+    function toHaveProperties(...args: any[]) {
+      toHaveProperties.calls++;
+      const options = args[args.length - 1];
+      expect(options).toHaveProperty('fn');
+      expect(options).toHaveProperty('inverse');
+      return 42;
+    }
+    toHaveProperties.calls = 0;
+
+    function toNotHaveProperties(...args: any[]) {
+      toNotHaveProperties.calls++;
+      const options = args[args.length - 1];
+      expect(options).not.toHaveProperty('fn');
+      expect(options).not.toHaveProperty('inverse');
+      return 42;
+    }
+    toNotHaveProperties.calls = 0;
+
+    const nonBlockTemplates = ['{{foo}}', '{{foo 1 2}}'];
+    const blockTemplates = ['{{#foo}}42{{/foo}}', '{{#foo 1 2}}42{{/foo}}'];
+
+    for (const template of nonBlockTemplates) {
+      expectTemplate(template)
+        .withInput({
+          foo: toNotHaveProperties,
+        })
+        .toCompileTo('42');
 
-  const nonBlockTemplates = ['{{foo}}', '{{foo 1 2}}'];
-  const blockTemplates = ['{{#foo}}42{{/foo}}', '{{#foo 1 2}}42{{/foo}}'];
+      expectTemplate(template).withHelper('foo', toNotHaveProperties).toCompileTo('42');
+    }
 
-  for (const template of nonBlockTemplates) {
-    expectTemplate(template)
-      .withInput({
-        foo: toNotHaveProperties,
-      })
-      .toCompileTo('42');
+    for (const template of blockTemplates) {
+      expectTemplate(template)
+        .withInput({
+          foo: toHaveProperties,
+        })
+        .toCompileTo('42');
 
-    expectTemplate(template).withHelper('foo', toNotHaveProperties).toCompileTo('42');
-  }
+      expectTemplate(template).withHelper('foo', toHaveProperties).toCompileTo('42');
+    }
+
+    const factor = process.env.AST || process.env.EVAL ? 1 : 2;
+    expect(toNotHaveProperties.calls).toEqual(nonBlockTemplates.length * 2 * factor);
+    expect(toHaveProperties.calls).toEqual(blockTemplates.length * 2 * factor);
+  });
 
-  for (const template of blockTemplates) {
-    expectTemplate(template)
+  it('should pass expected "this" and arguments to helper functions', () => {
+    expectTemplate('{{hello "world" 12 true false}}')
+      .withHelper('hello', function (this: any, ...args) {
+        expect(this).toMatchInlineSnapshot(`
+          Object {
+            "people": Array [
+              Object {
+                "id": 1,
+                "name": "Alan",
+              },
+              Object {
+                "id": 2,
+                "name": "Yehuda",
+              },
+            ],
+          }
+        `);
+        expect(args).toMatchInlineSnapshot(`
+          Array [
+            "world",
+            12,
+            true,
+            false,
+            Object {
+              "data": Object {
+                "root": Object {
+                  "people": Array [
+                    Object {
+                      "id": 1,
+                      "name": "Alan",
+                    },
+                    Object {
+                      "id": 2,
+                      "name": "Yehuda",
+                    },
+                  ],
+                },
+              },
+              "hash": Object {},
+              "loc": Object {
+                "end": Object {
+                  "column": 31,
+                  "line": 1,
+                },
+                "start": Object {
+                  "column": 0,
+                  "line": 1,
+                },
+              },
+              "lookupProperty": [Function],
+              "name": "hello",
+            },
+          ]
+        `);
+      })
       .withInput({
-        foo: toHaveProperties,
+        people: [
+          { name: 'Alan', id: 1 },
+          { name: 'Yehuda', id: 2 },
+        ],
       })
-      .toCompileTo('42');
-
-    expectTemplate(template).withHelper('foo', toHaveProperties).toCompileTo('42');
-  }
-
-  const factor = process.env.AST || process.env.EVAL ? 1 : 2;
-  expect(toNotHaveProperties.calls).toEqual(nonBlockTemplates.length * 2 * factor);
-  expect(toHaveProperties.calls).toEqual(blockTemplates.length * 2 * factor);
+      .toCompileTo('');
+  });
 });
 
 // Extra "blocks" tests

packages/kbn-handlebars/index.ts

@@ -392,17 +392,13 @@ class ElasticHandlebarsVisitor extends Handlebars.Visitor {
     decorator: hbs.AST.DecoratorBlock | hbs.AST.Decorator,
     prog: Handlebars.TemplateDelegate
   ) {
-    // TypeScript: The types indicate that `decorator.path` technically can be an `hbs.AST.Literal`. However, the upstream codebase always treats it as an `hbs.AST.PathExpression`, so we do too.
-    const name = (decorator.path as hbs.AST.PathExpression).original;
     const props = {};
-    // TypeScript: Because `decorator` can be of type `hbs.AST.Decorator`, TS indicates that `decorator.path` technically can be an `hbs.AST.Literal`. However, the upstream codebase always treats it as an `hbs.AST.PathExpression`, so we do too.
-    const options = this.setupParams(decorator as hbs.AST.DecoratorBlock, name);
-    // @ts-expect-error: Property 'lookupProperty' does not exist on type 'HelperOptions'
-    delete options.lookupProperty; // There's really no tests/documentation on this, but to match the upstream codebase we'll remove `lookupProperty` from the decorator context
+    const options = this.setupDecoratorOptions(decorator);
 
     const result = this.container.lookupProperty<DecoratorFunction>(
       this.container.decorators,
-      name
+      // @ts-expect-error: Property 'name' does not exist on type 'HelperOptions' - The types are wrong
+      options.name
     )(prog, props, this.container, options);
 
     Object.assign(result || prog, props);
@@ -642,31 +638,44 @@ class ElasticHandlebarsVisitor extends Handlebars.Visitor {
     };
   }
 
-  private setupParams(
-    node: ProcessableNodeWithPathParts,
-    helperName: string
+  private setupDecoratorOptions(
+    decorator: hbs.AST.Decorator | hbs.AST.DecoratorBlock
   ): Handlebars.HelperOptions {
-    const options: Handlebars.HelperOptions = {
-      // @ts-expect-error: Name should be on there, but the offical types doesn't know this
-      name: helperName,
-      hash: this.getHash(node),
-      data: this.runtimeOptions!.data,
-      loc: { start: node.loc.start, end: node.loc.end },
-    };
+    // TypeScript: The types indicate that `decorator.path` technically can be an `hbs.AST.Literal`. However, the upstream codebase always treats it as an `hbs.AST.PathExpression`, so we do too.
+    const name = (decorator.path as hbs.AST.PathExpression).original;
+    const options = this.setupParams(decorator as hbs.AST.DecoratorBlock, name);
 
-    if (node.params.length > 0) {
+    if (decorator.params.length > 0) {
       if (!this.processedRootDecorators) {
         // When processing the root decorators, temporarily remove the root context so it's not accessible to the decorator
         const context = this.scopes.shift();
         // @ts-expect-error: Property 'args' does not exist on type 'HelperOptions'. The 'args' property is expected in decorators
-        options.args = this.resolveNodes(node.params);
+        options.args = this.resolveNodes(decorator.params);
         this.scopes.unshift(context);
       } else {
         // @ts-expect-error: Property 'args' does not exist on type 'HelperOptions'. The 'args' property is expected in decorators
-        options.args = this.resolveNodes(node.params);
+        options.args = this.resolveNodes(decorator.params);
       }
     }
 
+    // @ts-expect-error: Property 'lookupProperty' does not exist on type 'HelperOptions'
+    delete options.lookupProperty; // There's really no tests/documentation on this, but to match the upstream codebase we'll remove `lookupProperty` from the decorator context
+
+    return options;
+  }
+
+  private setupParams(
+    node: ProcessableNodeWithPathParts,
+    helperName: string
+  ): Handlebars.HelperOptions {
+    const options: Handlebars.HelperOptions = {
+      // @ts-expect-error: Name should be on there, but the offical types doesn't know this
+      name: helperName,
+      hash: this.getHash(node),
+      data: this.runtimeOptions!.data,
+      loc: { start: node.loc.start, end: node.loc.end },
+    };
+
     if (isBlock(node)) {
       options.fn = this.generateProgramFunction(node.program);
       if (node.program) this.processDecorators(node.program, options.fn);