Next major version - Githubissues

line-o commented 1 year ago

The feedback from the community

[x] general acceptance
[x] make declarations for data-template="templates:parse-params" obsolete
[x] implement #20
[ ] implement #23 as it is a breaking change
[ ] forward port #24

NOTE: #22 is out of scope as it can be implement later as a configuration option

Embrace the top-down approach by allowing dynamic includes

with main page template, which is passed to templates:render

<html>
    <head>
        <title>${title}</title>
        <script data-template="templates:include"
            data-template-path="${includes?scripts}"></script>
        <style data-template="templates:include"
            data-template-path="${includes?styles}"></style>
    </head>
    <body>
        <!-- include template from module -->
        <nav data-template="templates:include"
            data-template-path="${includes?menubar}"></nav>
        <main data-template="templates:include"
            data-template-path="${includes?main}"></main>
    </body>
</html>

and a model, (the second parameter to templates:render)

map {
  "title": "My Page",
  "data": ((: this can be data loaded from the db for example :)),
  "includes": map {
    "scripts": "templates/includes/scripts.html",
    "styles": "templates/includes/styles.html",
    "main": "templates/pages/my-page.html",
    "menubar": "templates/includes/menubar.html"
  },
  "scripts": ( (: list of scripts to include in the page :) ),
  "styles": ( (: list of stylesheets to include in the page :) )
}

It is feasible to create quite flexible template structures that are easier to reason about. You do not need many, maybe no custom template function at all.

Don't worry templates:surround is still... around.

BREAKING CHANGES

The first parameter to templates:apply has to be a single node now. It was allowed to pass in more than one node but this was untested and the output would have been invalid XML anyway. So this "feature" was dropped.
The start and end delimiters for templates:parse-params (could be renamed to templates:substitute) must be set as configuration options now. This also means there is no way to change them somewhere in between.
The option to filter out template attributes from the output is now on by default. In order to include the in the output you will have to set $templates:CONFIG_FILTER_ATTRIBUTES : false() in the options.
The lib module was removed. Most of its functionality is now merged into the functions in the templates module. There is one exception: lib:resolve-apps is now part of a new module called tmpl-apps and the function was renamed to tmpl-apps:by-abbrev. It will put all resolved application paths into a map in the model under the apps key. Retrieval is done by <p id="test1">${apps?templating-test}</p>.

FEATURES

Function resolution

You can pass in xs:QName(?) as CONFIG_QNAME_RESOLVER instead of passing in a function that calls fn:function-lookup CONFIG_FN_RESOLVER. Due to a an issue in eXist-db's XQuery implementation (see https://github.com/eXist-db/exist/issues/4614 ) this can only resolve functions from imported modules at the time of writing. The old approach can be used as before. If you pass in none of the above the default resolver will still allow you to use all basic template functions in templates namespace.

Parameter resolution

The configuration option for parameter resolution can now contain a sequence of functions (function(xs:string) as item()*)* where it had to be a single function before. This allows for greater control over where parameter values are read from. Defaults to

(
    request:get-parameter(?, ()),
    session:get-attribute#1,
    request:get-attribute#1
)

Error reporting

when a template function cannot be resolved then the node is serialised in the error message to make it easier to find where the issue has occurred

New entry point

templates:render is a shorthand to templates:apply where the lookup is passed as part of the config

templates:render(
    <html><body></body></html>,
    map{},
    map {
        $templates:CONFIG_QNAME_RESOLVER : xs:QName(?),
        $templates:CONFIG_PARAM_RESOLVER : request:get-parameter(?,()),
        $templates:CONFIG_STOP_ON_ERROR : true(),
        $templates:CONFIG_FILTER_ATTRIBUTES : true()
    })

Max arity

With $templates:CONFIG_MAX_ARITY you can now specify the maximum arity of template functions. Defaults to 8 was 20.

Placeholder replacement

Before you had to declare data-template="templates:parse-params" (that was data-template="lib:parse-params" respectively in 1.1.0) to have placeholders in attributes and text nodes replaced. This is no longer necessary and placeholders will always be replaced using the provided resolvers or the model.

Changes

value retrieval => new public function templates:resolve-key($model, $key) will call the configured parameter resolvers internally
lots of refactoring for readability, maintainability and performance

As the tests show this version does raise the minimum required processor version to eXist-db v5.4.1 because of NPEs in v5.2.0.

duncdrum commented 1 year ago

@line-o please rebase

line-o commented 1 year ago

@duncdrum done! Thanks for merging the other PRs

joewiz commented 1 year ago

@line-o p.s. As long as this involves breaking changes, I'm not sure if you want to resurrect a previously reverted breaking change from @wolfgangmm involving the lib:parse-params() function? See the details here: https://github.com/eXist-db/templating/issues/8.

line-o commented 1 year ago

You are right @joewiz ! We should definitely include those in the next major version.

line-o commented 1 year ago

Also food for discussion is where to put which functions. As it is now possible to use only functions in the templates namespace, which functionality does it have to provide to be useful on its own?

[x] rendering a template (templates:render, templates:apply)
[x] loops (templates:for-each)
[x] subtemplates / partials / blocks (templates:include)
- this was moved to lib but I would like to reverse that or make lib dependency of templates
- include path should be possible from values in the model <a data-template="templates:include" data-template-path="model(page-template)" />
[x] output of values
- some directive to allow lookup in the model <a data-template="templates:text" data-template-text="model(link-text)" />
[x] lookup values in the model (see above)
- some way to tell the parameter resolution that this is a reference to a key in the model and has to be replaced with its value

line-o commented 1 year ago

@joewiz There is no need to reincorporate the reverted changes from #8 as the delimiters can no longer be set in templates but are now set once in the configuration.

https://github.com/eXist-db/templating/blob/a544af7c8c42190cc233a8d879406e89532dda06/test/xqs/test-suite.xql#L305-L306

If, however, this is not acceptable then we would have to look into this once more.

yamahito commented 1 year ago

Hello, sorry to be a bit late to the party!

I had some suggestions which I've previously raised with @joewiz for improving some aspects of the templating system; I've added them here as feature request #20 : is there any mileage in considering a similar solution as part of this next version?

Looking at the code, the implementation still looks fairly straightforward, with the suggested changes to templates:process now occurring in templates:process-element instead.

wolfgangmm commented 1 year ago

I wonder if it would make sense to have parse-params enabled by default, which means it would be active unless you stop it for some part of the tree, e.g. with disable-parse. This would align with other templating engines (e.g. in Nunjucks you use raw to stop substitution).

line-o commented 1 year ago

I appreciate your feedback, honestly. Good stuff here @duncdrum @wolfgangmm

re #20 (@yamahito): It's definitely not too late. Conditionals is one aspect that should be nailed in the standard-library, but I have not fully understood the proposed change and its implications, yet.
re documentation (@duncdrum): A complete overhaul is likely due. Would you be open to help on that?
re parse-params (@wolfgangmm): I agree that parsing placeholders should be transparent to the user of the library. Can you imagine a case where you would not want placeholders to be replaced? Would you be open for a discussion on how to implement this? Use expand-blocks instead of process-children?

duncdrum commented 1 year ago

@line-o i can help with the docs

line-o commented 1 year ago

@wolfgangmm your proposal is now part of this PR

line-o commented 1 year ago

I have an additional question regarding the handling of the now obsolete declaration of data-template="lib:parse-params". Should templates that are rendered with this new version of templating that still include it throw or the declaration just be ignored?

if we throw than the legacy declarations will certainly be removed in templates
if we ignore those templates might be usable in 1.x.x and 2.x.x versions of templating (just the templates not the view module)

Is it worth it keeping this extra check in the library or even harmful as those declarations stay in there? I am leaning towards throwing an error rather than ignoring the declaration.

duncdrum commented 1 year ago

+1 for throwing an error, explicit design >> black box design

line-o commented 1 year ago

@yamahito I decided to make a cut and implemented your original proposal with an additional error-check when both data-template-use-when and data-template-use-unless are set on a single element.

eXist-db / templating

Next major version #17