Feature: Support builder pattern in template engine #10

oscar60310 · 2022-05-25T06:47:26Z

01687ba Support query builder creation and execution.

{% req user %}
select * from users;
{% endreq %}
select * from group where userId = '{{ user.value()[0].id }}'

We'll traverse all nodes and find the .value() function call, and replace then with filters.

select * from group where userId = '{{ (user | execute)[0].id }}'

581cee4 5e0be39 Support main builder

{% req user main %}
select * from users;
{% endreq %}

a9bf698 8ca2533 Migrate packages to use builders: render() -> execute()

1cda034 4c4d038 Refactor: modulize extensions

Extensions now have two kinds: runtime and compileTime extensions.
Put the extensions with the same feature togeter instead of grouping by
types.
We only test with a full extension now, instead of test by files.
Metadata objects became a key-value mapping with extension name

  {
    "error.vulcan.com": {
      "errorCodes": [
        { "code": "QAQ", "locations": [{ "lineNo": 2, "columnNo": 9 }] }
      ]
    },
    "builder.vulcan.com": { "finalBuilderName": "FINAL_BUILDER" },
    "parameter.vulcan.com": [
      { "name": "users", "locations": [{ "lineNo": 1, "columnNo": 38 }] }
    ],
    "filter.vulcan.com": []
  }

719d8a9 throw error if different builders use same name

Besides some mock interfaces and functions, almost all code are covered by unit testing :D

kokokuo

Excellent , beside some question and suggestion, other is LGTM 👍

kokokuo · 2022-06-28T07:37:11Z

packages/core/src/lib/template-engine/nunjucksCompiler.ts

+      this.compileTimeEnv.extensionsList,
+      {}
+    );
+    this.traverseAST(ast);


Maybe we could let AST to Ast for consistency

Sure, fixed.

kokokuo · 2022-06-28T08:23:22Z

packages/core/src/lib/template-engine/built-in-extensions/query-builder/reqTagBuilder.ts

+    nextToken = parser.peekToken();
+    let mainBuilder = false;
+    if (nextToken.type !== lexer.TOKEN_BLOCK_END) {
+      if (nextToken.type !== lexer.TOKEN_SYMBOL || nextToken.value !== 'main') {


What is TOKEN_SYMBOL ( like % ) and why need to check is not TOKEN_SYMBOL ?
If it is a symbol but not main, you also make it a main builder default, too? ( If true, maybe we could give a comment for notice ? )

What is TOKEN_SYMBOL ( like % ) and why need to check is not TOKEN_SYMBOL ?

A symbol token is something like main, roughly speaking it is some char wich isn't contain "()[]{}%*-+~/#,:|.<>=!", not start with int ..... You can check here to see how it works.
I checked it must be SYMBOL or I threw error here to prevent following cases:

{% req main %} -- correct ---- incorrect {% req "main" %} {% req { main } %} ....

If it is a symbol but not main, you also make it a main builder default, too? ( If true, maybe we could give a comment for notice ? )

No, I won't. parser.fail will throw an error and the code below won't be executed.
I'll update the return of this function to "never" to indicate that:

- fail(message: string, lineno?: number, colno?: number): void; + fail(message: string, lineno?: number, colno?: number): never;

Thanks for replying to me and the logistic part I misunderstood, thanks !

kokokuo · 2022-06-28T08:29:57Z

packages/core/src/lib/template-engine/built-in-extensions/query-builder/reqTagBuilder.ts

+  private checkMainBuilder(node: nunjucks.nodes.CallExtensionAsync) {
+    const isMainBuilder =
+      node.extName === this.getName() &&
+      (node.args.children[1] as nunjucks.nodes.Literal).value === 'true';


Just would like to check my comprehension, Is here you check the true in the literal nod which from here and here you added in the child ?

Yes, the first argument indicates the variable name, and the second one indicates whether it is the main builder.

kokokuo · 2022-06-28T08:55:57Z

packages/core/src/lib/template-engine/built-in-extensions/query-builder/reqTagBuilder.ts

+    this.hasMainBuilder = true;
+  }
+
+  private wrapOutputWithBuilder() {


Just would like to check my comprehsion. Is this function used to wrap the builder from raw sql and the builder default is the main builder, like the below sample?

{% req user %} select * from users where id = '{{params.id}}' {% endreq %} select * from public.orders where userId = '{{ user.id.value() }}';

then I have a question when we face the below case, the not wrap req statement will still show the result? because since the main builder will output ?

{% req user main %} select * from users where id = '{{params.id}}' {% endreq %} select * from public.orders where userId = '{{ user.id.value() }}'; ----

I only check whether you have a main builder in your code to decide to send output or not, so:
1.

{% req user %} select * from users where id = '{{params.id}}' {% endreq %} select * from public.orders where userId = '{{ user.id.value() }}';

This statement will be wrapped, the finial query will be select * from public.orders where userId = 'xxxx';

{% req user main %} select * from users where id = '{{params.id}}' {% endreq %} select * from public.orders where userId = '{{ user.id.value() }}';

In this case, because we have defined main builder, the second query select * from public.orders where userId = '{{ user.id.value() }}'; won't be executed. But the first one select * from users where id = '{{params.id}}' will be executed twice (because .value() call)

Thanks for relying to me about the logistic question, thanks !

kokokuo · 2022-06-28T09:02:04Z

packages/core/src/lib/template-engine/extension-loader/models.ts

+    const args = originalArgs
+      .slice(1, originalArgs.length - 1)
+      .filter((value) => typeof value !== 'function');
+    const contentArgs = originalArgs


Maybe you could some comment to hint what is the difference between the args and contentArgs or give some sample to let us know because even saw the type in the TagRunnerOptions, still could not actually know what they are.

I found that there are some differences between these two arguments:

args will be passed to function directly: Literal("main") => extension.__run(context, "main"), our options of the extension should be set on these values.

contentArgs will be "rendered" and passed to function: Output('main') => t = ""; t += "main", the content we want the output should be set on these values.

I've updated the comments of the interface:

protected createAsyncExtensionNode( /** * The arguments of this extension, they'll be rendered and passed to run function. * It usually contains the configuration of the extension, e.g. {% req variable %} The variable name of req extension. * Note that these arguments will be pass to run function directly: Literal('123') => "123", so adding Output nodes causes compiling issues. Output("123") => t += "123" */ argsNodeList: nunjucks.nodes.NodeList, /** The content (usually the body) of this extension, they'll be passed to run function as render functions * It usually contains the Output of your extension, e.g. {% req variable %} select * from user {% endreq %}, the "select * from user" should be put in this field. * Note that these nodes will be rendered as the output of template: Output("123") => t = ""; t += "123", so adding nodes with no output like Symbol, Literal ... might cause compiling issues. Literal('123') => t = ""; 123 */ contentNodes: nunjucks.nodes.Node[] = [] )

Thanks, it will be more clear and thanks for searching the different parts and adding the rich comment 👍

- Add builder value visitor to modify AST - Add execute filter to call builder.execute() - Support replace callback while visit nodes' children

add a new argument (the second one) of extension, its value should be "true" or "false", indicating whether this builder is the final builder or not.

- Extensions now have two kinds: runtime and compileTime extensions. - Put the extensions with same feature togeter instead of grouping by types. - We only test with a full extension now, instead of test by files.

oscar60310 · 2022-07-01T09:38:23Z

Hi @kokokuo , all issues have been fixed.

kokokuo · 2022-07-01T10:28:15Z

It has been reviewed again, thanks for fixing and replying to me!

oscar60310 force-pushed the feature/builder-pattern-te branch 3 times, most recently from d9e8b9a to 66f161f Compare June 2, 2022 04:05

oscar60310 force-pushed the feature/builder-pattern-te branch 7 times, most recently from 993c66d to 719d8a9 Compare June 9, 2022 07:54

oscar60310 changed the title ~~[WIP] Feature: Support builder pattern in template engine~~ Feature: Support builder pattern in template engine Jun 9, 2022

oscar60310 requested review from kokokuo and wwwy3y3 June 9, 2022 08:13

oscar60310 marked this pull request as ready for review June 9, 2022 08:13

kokokuo mentioned this pull request Jun 27, 2022

Feature: Extensions support #6

Merged

kokokuo approved these changes Jun 28, 2022

View reviewed changes

oscar60310 force-pushed the feature/ioc-container branch from 01687ba to 1c5e76f Compare June 30, 2022 07:33

oscar60310 added 9 commits June 30, 2022 16:15

feat(core): query builder support in template engine

bf6976f

- Add builder value visitor to modify AST - Add execute filter to call builder.execute() - Support replace callback while visit nodes' children

feat(core): add "main" denotation of req extension

76fd094

add a new argument (the second one) of extension, its value should be "true" or "false", indicating whether this builder is the final builder or not.

feat(core): ads main builder visitor to validate and wrap builders

e91d652

fix(core): update compiler to use new builder

67181f9

test(core): add more test cases for extensions

797519b

refactor(core): extensions modulize

b82aa58

- Extensions now have two kinds: runtime and compileTime extensions. - Put the extensions with same feature togeter instead of grouping by types. - We only test with a full extension now, instead of test by files.

fix(build): fix dependencies of core package

165fa1f

feat(core): throw error if different builders use same name

2c13c1a

fix(core): delete test scripts and fix container load function calls

20a8dce

oscar60310 force-pushed the feature/builder-pattern-te branch from e878082 to 20a8dce Compare June 30, 2022 08:25

oscar60310 added 3 commits June 30, 2022 17:27

fix(core): fix function name for keep constent

4a27aeb

fix(core): make parser.fail return never instead of void

351f58d

fix(core): add descriptions and examples for extension options

4edd9e9

Base automatically changed from feature/ioc-container to develop July 1, 2022 10:29

kokokuo merged commit cda4f04 into develop Jul 1, 2022

kokokuo deleted the feature/builder-pattern-te branch July 1, 2022 10:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Support builder pattern in template engine #10

Feature: Support builder pattern in template engine #10

oscar60310 commented May 25, 2022 •

edited

Loading

kokokuo left a comment

kokokuo Jun 28, 2022

oscar60310 Jun 30, 2022

kokokuo Jun 28, 2022

oscar60310 Jun 30, 2022 •

edited

Loading

kokokuo Jul 1, 2022

kokokuo Jun 28, 2022 •

edited

Loading

oscar60310 Jun 30, 2022 •

edited

Loading

kokokuo Jun 28, 2022

oscar60310 Jun 30, 2022

kokokuo Jul 1, 2022

kokokuo Jun 28, 2022

oscar60310 Jul 1, 2022 •

edited

Loading

kokokuo Jul 1, 2022

oscar60310 commented Jul 1, 2022

kokokuo commented Jul 1, 2022

Feature: Support builder pattern in template engine #10

Feature: Support builder pattern in template engine #10

Conversation

oscar60310 commented May 25, 2022 • edited Loading

kokokuo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscar60310 Jun 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokokuo Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

oscar60310 Jun 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscar60310 Jul 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscar60310 commented Jul 1, 2022

kokokuo commented Jul 1, 2022

oscar60310 commented May 25, 2022 •

edited

Loading

oscar60310 Jun 30, 2022 •

edited

Loading

kokokuo Jun 28, 2022 •

edited

Loading

oscar60310 Jun 30, 2022 •

edited

Loading

oscar60310 Jul 1, 2022 •

edited

Loading