[@kbn/handlebars] Add support for partials

[watson]

Add support for partials to our own implementation of the handlebars template engine. The most important reason for this change is support for inline partials, which is a prerequisite for later changing default value of csp.disableUnsafeEval from false to true.

Closes #139068

Release notes

Added support for "inline partials" in handlebars templates when csp.disableUnsafeEval is set to true. This allows users who rely on inline partials to remove the unsafe-eval source expression in the Kibana Content Security Policy (CSP).

[github-actions]

Documentation preview:

• ✨ Changed pages

[watson]

Current CI failures are unrelated to this PR as main is currently not passing CI.

[azasypkin]

Code LGMT! I couldn't confidently review all intricacies of the Handlebars logic due to lack of the specific knowledge, but as I said elsewhere, for this specific fork it feels reasonable to rely on the extensive test coverage that you provided.

[azasypkin]

optional nit: I see we have multiple places in this function where we rely on !, wondering if it'd make sense to move this.runtimeOptions! to a local variable and use it instead? Unless I'm missing something...

      // It should be defined because ...
      const currentRuntimeOptions = this.runtimeOptions!;
runtimeOptions.data = runtimeOptions.data || currentRuntimeOptions.data;
      if (runtimeOptions.blockParams) {
        runtimeOptions.blockParams = runtimeOptions.blockParams.concat(
          currentRuntimeOptions.blockParams
        );
      }

      // inherit partials from parent program
      runtimeOptions.partials = runtimeOptions.partials || currentRuntimeOptions.partials;

[watson]

Yeah, it's because runtimeOptions isn't set in the constructor, but in render, which technically means it's possible to get to this point in the code before it's set. But only theoretically as render is ALWAYS called before anything else, so we can just treat it as if it is always set.

We use this.runtimeOptions! all over the code, not only in this function, so I would prefer an approach that satisfied the entire class than to simply add a local variable in each function. Otherwise I prefer the ! approach.

[azasypkin]

question: Is there a valid use case for name to be a number?

[watson]

Yes it seems odd, but check out this test: https://github.com/elastic/kibana/pull/150151/files#diff-2e0dbeea10dcf18ef360e3f510487442df799be06fc2830afa55a3711ddc04ecR245-R250

[azasypkin]

question: Cannot Jest's toThrow (inside test_bench) perform instanceof check? https://jestjs.io/docs/expect#tothrowerror

[watson]

Unfortunately only if you give up checking the message. You can either do:

.toThrow('The partial missing could not be found')

Or:

.toThrow(Handlebars.Exception)

You could technically do:

.toThrow(new Handlebars.Exception('The partial missing could not be found'))

But that is according to my research technically equivalent to just checking the string value as it doesn't seem to perform an instanceof check.

But maybe I'm missing a completely other option. However, it's not super important.

[watson]

I guess I could make some custom logic that after the toThrow check, does a expect(e.message).toEqual(...) check 🤷 (but that quickly becomes overly complicated, which is probably why I didn't do it)

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 7d56921

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
@kbn/handlebars	12	14	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
canvas	1022.3KB	1023.9KB	+1.7KB
uiActionsEnhanced	135.1KB	136.8KB	+1.7KB
visTypeTimeseries	506.2KB	507.9KB	+1.7KB
total			+5.1KB

Unknown metric groups

API count

id	before	after	diff
@kbn/handlebars	19	21	+2

ESLint disabled line counts

id	before	after	diff
@kbn/handlebars	4	5	+1

Total ESLint disabled count

id	before	after	diff
@kbn/handlebars	8	9	+1

History

• 💚 Build #105056 succeeded 2020abc
• 💔 Build #105033 failed e21d385

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @watson

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

[elasticmachine]

Pinging @elastic/kibana-security (Team:Security)