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

"Require" Pattern for Actual Reusable Dynamic Functions Alongside Traditional Includes #328

Closed
DGruenwald opened this issue Dec 14, 2017 · 6 comments

Comments

@DGruenwald
Copy link

DGruenwald commented Dec 14, 2017

When it comes to dynamic reusable snippets, I feel like the current include system is a bit lacking:

  • You can only have one "function" per file.
  • You have to pass in an object, meaning you have to name all arguments when calling.
  • Parameters aren't automatically declared, leading to "x is undefined" errors, or worse, causing values to "leak" in from the outside.
  • No other built-in benefits of using functions like the 'arguments' variable for example.

I think the 'require'-pattern would be much better for this:

// -- utils.ejs --

<% exports.time = function (date) { _%>
  <time datetime="<%= date.toISOString(); %>"><%= date.toString(); %></time>
<% }; _%>
<% exports.list = function () { _%>
  <ul>
    <% Array.from(arguments).forEach((item) => { _%>
      <li><%= item %></li>
    <% }); _%>
  </ul>
<% }; %>

// -- view --

<% var utils = require("utils"); %>
<% utils.time(new Date()); %>
<% utils.list("A", "B", "C"); %>

My solution would be adding a require() function to the local scope, that works similar to include(), but returns an object or function instead of a compiled string.
This wouldn't break anything, since both include and require can exist without problems.

On top of that, it isn't actually that difficult to implement, as it's just a matter of passing down the '__output' array and returning an exports object instead of the joined array.
In fact, I've already successfully implemented it in a local copy and I just wanted to ask here first before I create a pull request.

@User4martin
Copy link

This seems similar to snippets https://github.com/User4martin/ejs/blob/plugin-opts-filter/docs/plugin-snippet.md
#295

If you want to return data from an include you do not need require. You can add then to an object on the locals variable.

<% locals.utils = {} %>
<%- include('foo')  %>

foo can add keys to locals.utils.

foo is given a flat copy of locals. Therefore the caller will not see changes made to locals directly. However locals.utils is a reference, and foo gets the same reference, effectively allowing to add/modify keys in utils and those changes being visible to the caller.

@DGruenwald
Copy link
Author

DGruenwald commented Dec 20, 2017

Nope, that doesn't work since every include gets its own '__output' array which means when you actually call functions declared inside the include, everything gets appended to the wrong array, meaning you can't use any html and ejs-tags inside them, defeating the whole purpose of ejs.

I don't want to return data, but reusable real functions that can utilize ejs tags.

Also, that snippets thing you linked to: That suffers from some of the same problems I've listed above. Why use something more limited and needlessly complex, when you could just as easily use functions?

@mde
Copy link
Owner

mde commented Jan 3, 2018

Some thoughts here:

There are lots of different ways to think about code reuse in a templating system. You are arguing for a functional approach. Some people advocate for template inheritance, which brings in a class-based approach.

I prefer to think of this in terms of content reuse, and don't think we should need a lot of programming modularity inside of our templates. (I agree with the Mustache folks on this -- I just think inventing yet another programming language to do it is very silly.) For a templating system, the ability to include recursively the content of one rendered template inside of another is sufficient.

(I would also argue that parameters should not be automatically declared. Being more explicit is generally good when you're moving between contexts (from your top-level handler function, into the template, for instance).

However, if you really dig the functional approach -- what you're describing here actually sounds a lot like tagged template literals (see midway down the page here):

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals

And if you're intent on using EJS syntax rather than simple interpolation, you could easily wrap an EJS render inside a tagged template literal.

This is not something we'd implement in EJS core. I'm going to close this issue, but happy to reopen if further discussion is warranted.

@mde mde closed this as completed Jan 3, 2018
@DGruenwald
Copy link
Author

DGruenwald commented Jan 3, 2018

I feel like you're more arguing about philosophy and the fact that I called it "require-style-includes" instead of "requiring files" or something here.
From a practical point of view though, as I said, I think the current include system can sometimes be a pain (and, in my opinion, is not "sufficient"), and this would definitely help with that.

Look, isn't one of ejs main selling points that it's just Javascript and that it just works like you'd expect? That just isn't the case when it comes to includes and functions.
When you create a function inside an include, first of all it's completely counter intuitive to even get it out of it's context (see above), second of all, when you actually use it, absolutely nothing happens, which will probably throw a lot of people for a loop.

And seeing as ejs is just Javascript: I'm not introducing some completely crazy new ideas here either, in fact, what I'm using here, is one of the very basic building blocks of Javascript and something everyone should be familiar with when working with node.

If you look at some of the other issues around here you can clearly see that "blocks," "snippets" or whatever you want to call them are something that ejs lacks and I just don't see why you have to invent a complex new system, when actual functions could do the same job and probably better.

At the end of the day, do you really want to reinvent functions (but likely not as good), just so you don't have to call them functions?

Also, in my opinion, that parameters are not automatically declared can be a big problem, because it means that you have to be always aware of the contents of your 'locals'-object to not leak in values, which means "being explicit" isn't even optional anymore.
Plus, unless I add a comment, at a glance I have no clue what arguments it is expecting and it makes optional arguments more complex since, as I said, it leads to "x is undefined" errors.

Lastly, I don't know if you've seen my pull request (#329), but it's literally just a 20-line change, that doesn't impact any other aspect of ejs and doesn't break older code either.

Both "philosophies" that include() and require() represent can happily co-exist without any problems and I feel like in this case having more options at your disposal doesn't have any downsides at all.

@DGruenwald
Copy link
Author

@mde Yes, I would like to discuss this further. See above.

@DGruenwald
Copy link
Author

@mde I'm still waiting on more input.

@DGruenwald DGruenwald changed the title Require-Style Includes "Require" Pattern for Reusable Functions Alongside Traditional Includes Jul 19, 2021
@DGruenwald DGruenwald changed the title "Require" Pattern for Reusable Functions Alongside Traditional Includes "Require" Pattern for Actual Reusable Dynamic Functions Alongside Traditional Includes Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants