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

Fix for nested nunjucks async shortcodes #1749

Merged
merged 3 commits into from Nov 20, 2021
Merged

Conversation

CodeFoodPixels
Copy link
Contributor

Nested async shortcodes would fail to correctly output content from the inner shortcode as eleventy wasn't waiting for the content to have processed before passing it to the parent.

The body() call takes a callback, which gets called when the passed content of a paired shortcode has finished rendering, both on async and sync templates.

Fixes #1053

@@ -287,57 +287,68 @@ class Nunjucks extends TemplateEngine {
var body = parser.parseUntilBlocks("end" + shortcodeName);
parser.advanceAfterBlockEnd();

if (isAsync) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I’m reading this right, this makes all Nunjucks paired shortcodes into Async shortcodes? Is this slower?

@CodeFoodPixels
Copy link
Contributor Author

Thinking about it, this is probably an issue with Nunjucks itself

@CodeFoodPixels
Copy link
Contributor Author

I've created a failing test and submitted it to Nunjucks: mozilla/nunjucks#1363

@zachleat
Copy link
Member

Maybe related 11ty/eleventy-img#110

@zachleat
Copy link
Member

I’m interested in trying to fix this in Eleventy, if possible. I don’t think mozilla/nunjucks#1357 is going to be enough, in local testing.

@zachleat
Copy link
Member

Filed #2108 which is now the home base issue for this PR, I think.

@zachleat
Copy link
Member

This is a beautiful PR and I’m so sorry I slept on it for too long.

It doesn’t quite complete #2108 but it takes us a step in the correct direction.

As stated previously there may be some (as of now untested) performance drawback to making everything async but we need to err on the side of correctness over performance here.

Shipping.

@zachleat zachleat added this to the Eleventy 1.0.0 milestone Nov 20, 2021
@zachleat
Copy link
Member

zachleat commented Nov 20, 2021

This will ship with 1.0.0-beta.9

@zachleat zachleat merged commit fae14a5 into 11ty:master Nov 20, 2021
zachleat added a commit that referenced this pull request Nov 20, 2021
zachleat added a commit that referenced this pull request Nov 20, 2021
…sh between async and sync shortcodes in Nunjucks now, due to #1749.
zachleat added a commit that referenced this pull request Dec 19, 2021
…istinguish between async and sync shortcodes in Nunjucks now, due to #1749."

This reverts commit f3831c3.
@zachleat
Copy link
Member

zachleat commented Jan 9, 2022

For future me, note that the fix here was included in 1.0 stable but the async-everything approach was reverted in #2140.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nunjucks: async shortcode inside paired shortcode
2 participants