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.

Sign up for GitHub

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

Jump to bottom

issue in include() functionality #254

Open
morandd opened this issue Aug 15, 2023 · 6 comments
Open

issue in include() functionality #254

morandd opened this issue Aug 15, 2023 · 6 comments

Comments

@morandd
Copy link

morandd commented Aug 15, 2023

Describe the bug

Nested includes() do not pass data to children templates by default.
If you use:
<%~ include("./child.eta") %>
then child.eta will not receive any data available at the parent. But, if you provide an empty object as the second parameter to include, like this:
<%~ include("./child.eta", {}) %>
then the child does inherit access to the parents data.

The documentation implies that the second data parameter is optional and provides extra variables that will be merged with "it". But the observed behaviour is that the second parameter is undefined and thus overwrites "it".
I think the issue arises as line 23 of compile-string.ts.

I suggest that nested child includes()'s should have access to the parent's data by default as this makes sense intuitively and is the behaviour of EJS.

Thanks for your contribution to this. Deno needs a good templating engine.
@nebrelbug
Copy link
Collaborator

@morandd thanks for opening this issue!

But, if you provide an empty object as the second parameter to include, like this:
<%~ include("./child.eta", {}) %>
then the child does inherit access to the parents data.

Are you sure this is the case? Though this should happen (according to documentation 😅), looking at the code, it doesn't seem like the data is merged with it at all.

@morandd
Copy link
Author

morandd commented Aug 16, 2023 •
edited
Loading

Yes, I think I can confirm this is a bug. Here's a test case:

templates/mypage.eta template page:

--- start of mypage.eta ---
<%~ 
/* According to 'merge' principle in the documentation, header.eta should receive it.lang and it.name */
include('header.eta',{name:'Ben'}); %>
--- end of mypage.eta ---

which uses the header templates/header.eta partial:

--header.eta
<html lang="<%~ it?.lang%>">
hi I am a header
<tag lang="<%~ JSON.stringify(it) %>">
--end header.eta

And we execute it using a simple script test.js:


import { Eta } from "https://deno.land/x/[email protected]/src/index.ts";
const eta = new Eta({ views:  Deno.cwd() + '/templates' })
console.log( eta.render("mypage.eta", { lang: "en" }) );

The If the merge operation happened correctly then header.eta should have access to it.lang and it.name, but it only has it.lang. Exectuing test.js produces:

--- start of mypage.eta ---
--header.eta
<html lang="undefined">
hi I am a header
<tag lang="{"name":"Ben"}">
--end header.eta--- end of mypage.eta ---

@morandd morandd mentioned this issue Aug 16, 2023
Closed
@morandd
Copy link
Author

morandd commented Aug 16, 2023

a workaround is to add {it} to every child include() call. For example:

<%~ include('header.eta',{it}) %>

this way the child partial header.eta will receive it from the parent.

@nebrelbug
Copy link
Collaborator

@morandd thanks for the thorough details and tests. This begs the question: do we want to automatically have partials inherit it from their parents, or should we remove that behavior from the docs?

I'll open up the discussion on Discord.

@bacloud22 bacloud22 mentioned this issue Oct 2, 2023
Closed
@paul-norman
Copy link

I've just started porting an EJS project to ETA and this is causing quite a lot of extra code. The docs state that:

"we can also pass in data that will be merged with it and passed to the partial"

But, as noted above by Ben, this is not in fact implemented at all.

Oddly, the layout() function does seem to work like this by default (and would be quite weird if it didn't), but the include / includeAsync functions are missing the ability.

At the moment, I am basically writing something like:

<%~ include('@template', Object.assign({}, it, { extra: 123 })) %>

almost everywhere (out of laziness - because I need a handful of global variables in almost all partials). This is leaving me with very messy template files. It would be nice to either have a third parameter that tells the function to use the global data value, or a separate set of functions that work this way:

<%~ include('@template', { extra: 123 }, true) %>
// or
<%~ includeData('@template', { extra: 123 }) %>

I prefer the second approach because it allows seemless passing without an empty object if there is no extra data to pass:

<%~ include('@template', {}, true) %>
// vs
<%~ includeData('@template') %>

Of course the includeAsync functions would also need this ability too.

@nebrelbug
Copy link
Collaborator

@paul-norman I understand where you're coming from. However, there is a faster way to merge data. It seems like { ...it, extra: 123 } might not add that much code and might make it more obvious for users how data is flowing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@paul-norman @morandd @nebrelbug