Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to run lambdas as plugin mustache extensions #951

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 53 additions & 4 deletions mwdb/web/src/components/RichAttribute/MarkedMustache.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from "lodash";
import Mustache, { Context } from "mustache";
import { marked, Tokenizer } from "marked";
import {
Expand All @@ -6,6 +7,7 @@ import {
renderTokens,
Token,
} from "@mwdb-web/commons/helpers";
import { fromPlugins } from "@mwdb-web/commons/plugins";

/**
* Markdown with Mustache templates for React
Expand All @@ -24,6 +26,10 @@ function appendToLastElement(array: string[], value: string) {
return [...array.slice(0, -1), array[array.length - 1] + value];
}

function isFunction(obj: Object): boolean {
return typeof obj === "function";
}

Comment on lines +29 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use lodash _.isFunction for that, definition is the same 😄 https://github.com/lodash/lodash/blob/a67a085cc0612f5b83d78024e507427dab25ca2c/src/isFunction.ts#L28

Meanwhile I found that lodash is not really maintained anymore 🤔

function splitName(name: string) {
if (name === ".")
// Special case for "this"
Expand Down Expand Up @@ -112,7 +118,12 @@ class MustacheContext extends Mustache.Context {
}
if (!name) return undefined;
const path = splitName(name);
let currentObject = this.view;
// In case of lambdas, the subrenderer makes
// this.view be a MustacheContext so make sure
// we get the actual view
let currentObject = this.view instanceof MustacheContext
? this.view.view
: this.view;
Comment on lines +121 to +126
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's caused by incorrect overload of Mustache.Writer.render, I didn't know that it's called recursively (https://github.com/janl/mustache.js/blob/972fd2b27a036888acfcb60d6119317744fac7ee/mustache.js#L593).

I think the best solution is to create root MustacheContext in renderValue instead of overloading render

for (let element of path) {
if (!Object.prototype.hasOwnProperty.call(currentObject, element))
return undefined;
Expand All @@ -122,8 +133,8 @@ class MustacheContext extends Mustache.Context {
this.lastValue = currentObject;
if (searchable) {
if (
typeof currentObject === "object" ||
typeof currentObject === "function"
isFunction(currentObject) ||
typeof currentObject === "object"
)
// Non-primitives are not directly searchable
return undefined;
Expand All @@ -135,6 +146,9 @@ class MustacheContext extends Mustache.Context {
if (!query) return undefined;
return new SearchReference(query, currentObject);
}
if (isFunction(currentObject)) {
currentObject = currentObject.call(this.view);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that object returned by lookup is already called by Mustache.js as a part of renderSection. Do we need to resolve it at lookup level?

https://github.com/janl/mustache.js/blob/master/mustache.js#L609

}
return currentObject;
}
}
Expand Down Expand Up @@ -212,8 +226,43 @@ class MarkedTokenizer extends Tokenizer {
const mustacheWriter = new MustacheWriter();
const markedTokenizer = new MarkedTokenizer();

type stringOpFunc = (I: string) => string;

const lambda = (func: stringOpFunc = _.identity) => {
// A factory method for custom lambdas.
//
// The inner method receives the text inside the section
// and the subrenderer. It then renders the value, and passes
// it to a user defined function.
//
// E.g.: ("{{name}}", renderer) => "John Doe"
// (func is toUpperCase) => "JOHN DOE"
return () => function(text: string, renderer: any): string {
return func(renderer(text.trim()));
}
};

const lambdas = fromPlugins("mustacheExtensions").reduce(
(prev, curr) => {
return {
...prev,
..._.mapValues(curr, (func: stringOpFunc) => lambda(func))
}
},
{});

Comment on lines +245 to +253
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Global scope is too early for call to fromPlugins as the list of plugin extensions is loaded lazily during initial render (https://github.com/CERT-Polska/mwdb-core/blob/master/mwdb/web/src/index.jsx#L19)

I think it should be moved to the renderValue scope.

export function renderValue(template: string, value: Object, options: Option) {
const markdown = mustacheWriter.render(template, value);
const markdown = mustacheWriter.render(
template,
{
...value,
"value":
{
...(value as any).value,
...lambdas
}
}
);
Comment on lines +255 to +265
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value doesn't have to be a dictionary object e.g. List of entries example doesn't render correctly. It's better to put lambdas in top-level view object.

Suggested change
const markdown = mustacheWriter.render(
template,
{
...value,
"value":
{
...(value as any).value,
...lambdas
}
}
);
const markdown = mustacheWriter.render(
template,
{
...value,
...lambdas
}
);

The best solution would be a dedicated collection in MustacheContext.

const tokens = marked.lexer(markdown, {
...marked.defaults,
tokenizer: markedTokenizer,
Expand Down
Loading