From 1a94e763a7371ad6ac576a687175292eecaa0e31 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 19 Dec 2022 15:45:03 -0500 Subject: [PATCH] [8.6] [@kbn/handlebars] Ensure only decorators have an `options.args` property (#147791) (#147799) # Backport This will backport the following commits from `main` to `8.6`: - [[@kbn/handlebars] Ensure only decorators have an `options.args` property (#147791)](https://github.com/elastic/kibana/pull/147791) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) Co-authored-by: Thomas Watson --- packages/kbn-handlebars/index.test.ts | 145 +++++++++++++++++++------- packages/kbn-handlebars/index.ts | 49 +++++---- 2 files changed, 135 insertions(+), 59 deletions(-) diff --git a/packages/kbn-handlebars/index.test.ts b/packages/kbn-handlebars/index.test.ts index 9d255bf676746d..82f837a6b333f5 100644 --- a/packages/kbn-handlebars/index.test.ts +++ b/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 diff --git a/packages/kbn-handlebars/index.ts b/packages/kbn-handlebars/index.ts index a7ad36a9e8663e..9f8256a3bb5164 100644 --- a/packages/kbn-handlebars/index.ts +++ b/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( 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);