block/mixin (again) - Githubissues

User4martin commented 7 years ago

I am aware of #251 and #252

Please have a look at: https://github.com/User4martin/ejs/tree/template_subclass

This is not production ready / just proof of concept

It entirely optional:

It does not increase the size of the main ejs.js file / comment #251#issuecomment-294512048 about client side use
If not activated, then no syntax changes, no existing ejs gets broken. If not activated, then there is no performance change either.

The main change to ejs is to make the Template class available, and allow to substitute it, This will allow for any sort of plugin. (And maybe Template.modes.* should be exported separately (or on the prototype), so subclasses do not need to copy them)

I would (for better maintenance) like to see the file (when ready) included in the distribution. And maybe plugins for other features too. But if not then at least make plugins possible by the changes in ejs.js itself.

About the ejs-mixin

This is fully up for discussion.

The syntax for defining blocks can be changed if anyone has a better idea.
The use of "include()" can and should be replaced.

About the "include": There should probably be a <% mixin(name, locals) %> call instead. That can be done, but it would need some more changes to ejs.

The Template class would need a function, that returns a list of name/function pairs, that can be used.

Template.prototype.commands = function(data, opts) {
    return { inline: function()... {}, };
}

and there could be an additional prepended += ' with (argument_commands || {}) {' + '\n';

That way sub classes can add commands.

mde commented 7 years ago

A good step toward making EJS extensible, which might give us a better answer for when people have specific functionality they think needs to be added. Would be happy to work with you for some forward progress on this.

User4martin commented 7 years ago

Great, looking forward.

I will look at how to implement my "mixin", and also try to port ejs-locals to an add-on of ejs. I will revert when I have more of a demo.

I may have some other ideas of changes/improvements along the way. But I will open separate issues, if any of them materializes.

mde commented 7 years ago

One tiny thought: "mixin" has some fairly specific meaning already:

https://en.wikipedia.org/wiki/Mixin

Basically a bag of properties included (in JS, often copied onto) in an object, without being inherited from a parent class.

I would want to see how include is changed, but mixin doesn't seem like the right terminology.

(Worth nothing also that 'template inheritance' is a thing that exists, too, but seems very different from the 'inheritance' here where we're talking about subclassing the base Template.)

User4martin commented 7 years ago

I pushed an update version for discussion.

I renamed it to "snippet". I know "block" would be best. But that is used by ejs-locals (which I want to port to a plugin too). So I don't want to call it "block". The name "template" is already used to refer to the entire template, so it would be confusing too. And otherwise I am out of ideas for a good name.

The overall changes in lib/ejs.js are minimal (IMHO). Also the effect on compilation time is very low (on my system it is not measurable, since it is lower that the variance between to benchmarks of the same code.)

One question upfront: Would you be willing to include the code (as separate js file) in the main distribution? the following code in lib/ejs.js depending on that (if that concept is ok at all)

exports.enableSnippets = function () {
  require('./ejs-snippet');
};

Otherwise people would just need to require the plugin separately, and the plugin adds itself to the ejs module.

The actual changes needed in lib/ejs.js

exports.Template = Template; to make the class accesible
add an init method for subclasses
add the methods generateArgumentNames and generateArguments. Those allow plugins to change the argument list of the generated function.

With this the plugin can now pass in a parameter name "snippet". So in the template `<%- snippet('foo') %> can call the snippet function.

I also modified the var returnedFn = function so when an include is called, it also receives the generateArguments from the caller (it can then decide to generate new one, or keep them).

That way the top level template can generate ONE function for "snippet", and attach data to that function. And then all included (and nested included) templates can keep the same "snippet" function and data.

This means that a template defined in one file (e.g. "snips.ejs") can be used

in files included by "snips.ejs"
in files that included "snips.ejs"

As for the plugin itself: Current behaviour (which can be changed, but is actually convenient) should be (I still need to write the tests....):

Snippets are hoisted within the template in which they are defined.
- Snippets see the local data, as it was on top of the file (but like include can be given changes to the locals at each call)
Snippets defined in an include, are available only after the include was executed (after all the filename may be computed at run time)
Snippet definition can not be nested
- However a snippet can include other files, and in those other files new snippets can be defined

Any comments?

User4martin commented 7 years ago

I added tests, and it works all as expected.

I also made a 2nd version, based on the changes I made in other pull requests (and branches). https://github.com/User4martin/ejs/tree/template_subclass_regex

This version introduces a new mode <%* for control tags.

The mode is added from within the plugin.
Other plugins can share this mode.
Usage of the mode with unknown content is detected and gives an error <%* what %> will give an error at compile time.

User4martin commented 7 years ago

OFF TOPIC

I know I added a lot of pull requests (and have some more coming / see branches in my fork).

And probably the expected way would have been, to first check with you, if those changes are within the plans that you have for ejs.

However I wanted to first see, if my ideas work as expected (and benchmark them).

So I am aware that some of them may not be welcome. In which case let me know, so I can rebase the others. (If any remain, but I hope so).

Btw the https://github.com/User4martin/ejs/tree/optimize-combine-append-calls https://github.com/User4martin/ejs/tree/refactor-generate-source-full-tokens together with gives some good speed gain on execution time of the generated function. It depends a lot of the template. For some it will do nothing. One of the benchmarks gained ~ 40% on my machine (Though that template benefits more than usual, as it has a lot of literals <%% (

User4martin commented 7 years ago

@mde @RyanZim

I added docs for plugin api and snippets https://github.com/User4martin/ejs/blob/plugin-snippets/docs/plugins.md https://github.com/User4martin/ejs/blob/plugin-snippets/docs/plugin-snippet.md

Any comments would be welcome.

As far as I can see the snippets are ready as they are. (docs, test, code)

I am working on another plugin (replacing ejs-locals). This would bring a few minor modifications to ejs. But more later.

mde / ejs

block/mixin (again) #274