Replies: 7 comments 8 replies
-
Thanks Miro. To be honest, we didn't realize anyone else was writing custom
Nunjucks tags — it's pretty advanced stuff!
So to summarize, I think you are saying that any custom tags with a body
need to be written as async tags, otherwise that body won't work if it
touches any async capabilities of Apostrophe, which makes sense.
Is there any more to it than that?
…On Thu, Apr 15, 2021 at 3:13 PM Miro Yovchev ***@***.***> wrote:
The problem
I had a discussion with the core team about the troubles I'm facing with
brand new Apostrophe 3 project, related to newly introduced in the new apos
version nunjucks async extensions. I've spent a lot of time to dig and find
out what goes wrong (templates failing to render in a very weird ways) or
possibly could go wrong in the future in my journey with the the async
nunjucks components.
The tests
As a result I've created a repository,
<https://github.com/myovchev/a3-async-test> based on the latest A3
boilerplate, enhanced with a large number of visually tested code samples.
You just need to clone the repo and boot the project on the development
machine. Everything is documented (code) and explained (starting from the
home page).
I plan to keep this repository around, to expand it and test future
versions/related features of Apostrophe.
The summary
If you don't want to dig that deep, here is the summary.
- Don't add/use extensions (custom tags) on outside of Apostrophe,
especially if you see them register via CallExtension and not
CallExtensionAsync.
- When you do your custom tags (preferably via the customTags as shown
in the test project) and they have body (closing tag(s)), be sure to await
the body block(s) as it is done here
<https://github.com/myovchev/a3-async-test/blob/main/modules/test-case/lib/async-tags.js#L30>.
I think this deserves a special place in the core documentation.
- Go and install the test repo, take a look at those red fails and
check the corresponding template. This could save you a lot of time. I'm
talking from experience here.
Cheers.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2914>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAH27L5AHFZMAX273FU2HLTI43FPANCNFSM43AC3EAA>
.
--
THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
|
Beta Was this translation helpful? Give feedback.
-
Yes, when we get to documenting the `customTags` feature we should cover
these topics with good examples and clear warnings.
…On Fri, Apr 16, 2021 at 3:59 AM Miro Yovchev ***@***.***> wrote:
Additionally, Nunjucks docs are completely confusing as they mix async
context with the loader (e.g. load templates in async way, e.g. from the
db). The async chain of body described above is not documented or at least
I wasn't able to find it no matter I spent a lot time on their docs page.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2914 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAH27IRQ2RPYNO5FENKFZ3TI7U5NANCNFSM43AC3EAA>
.
--
THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
|
Beta Was this translation helpful? Give feedback.
-
Sadly this story is not ended yet. I've started a real work and refactoring of sync tags to async
Now my options are limited and I decided to go with includes (converting the macros who are calling custom tags). This hurts the design of the system I'm building and if the both checkboxes above are true, I don't even have much choice there (and time to convert around 50 macros and growing to fragments). Additionally I really think this all is a recipe for disaster. Everything above makes me think your initial decision for removing macros support was right one with the system stability in mind. But the whole feels wrong and not having macros is something I'm not sure I can live with. I also suppose a lot of projects (moving from A2 including) would have problem with that too. It sounds to me the CLEANEST solution here is a push towards Nunjucks core macros fix. I don't have any technical details and eventual complications regarding solving this in the Nunjucks core, but I'm ready to help with starting a conversation with Nunjucks core dev team and providing all relevant information, testcases and arguments about the critical importance of this subject. |
Beta Was this translation helpful? Give feedback.
-
Fragments should be able to invoke other fragments, if that doesn't work
then we clearly have something we need to fix, hopefully on our end and not
upstream.
…On Mon, Apr 19, 2021 at 2:32 PM Miro Yovchev ***@***.***> wrote:
Sadly this story is not ended yet. I've started a real work and
refactoring of sync tags to async customTags. And it slowly becomes a
nightmare so I need your input on the new findings.
1. There is a strange, yet not reproduced edge case when some (deep)
includes with their own extend (different from the main page layout
tree) refuse to render async content. Funny enough a dummy asyncComponent
before the include in question fixes everything. I'll come back with more
info and test cases ASAP.
2. I've converted custom tags to async and it immediately blew up in
my face, because some macros was using the sync tags, which now is a
problem. And this is a huge topic so I'll split it. It's about macros and
fragments. @boutell <https://github.com/boutell> Tom please mark those
points which are true and correct me if I'm wrong in case I'm missing
something.
- fragment functions can't call other fragment function even in the
same context (file) so converting directly macro which uses custom async
tag and is composed from more _private macro functions to fragments is
not possible.
- I can't import other macros and mix them in fragments, which makes
my only option to convert all macros to fragments (in case the above is not
true), even those who are just simple stateless components. So for example
the {{ buttons.primary() }} (a macro) is not possible inside any
fragment. In fact importing a macro inside a template having fragment is
leading to fatal error.
Now my options are limited and I decided to go with includes (converting
the macros who are calling custom tags). This hurts the design of the
system I'm building and if the both checkboxes above are true, I don't even
have much choice there (and time to convert around 50 macros and growing to
fragments). Additionally I really think this all is a recipe for disaster.
Everything above makes me think your initial decision for removing macros
support was right one with the system stability in mind. But the whole
feels wrong and not having macros is something I'm not sure I can live
with. I also suppose a lot of projects (moving from A2 including) would
have problem with that too.
It sounds to me the CLEANEST solution here is a push towards Nunjucks core
macros fix. I don't have any technical details and eventual complications
regarding solving this in the Nunjucks core, but I'm ready to help with
starting a conversation with Nunjucks core dev team and providing all
relevant information, testcases and arguments about the critical importance
of this subject.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2914 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAH27OBIJSULF5Y4I6QZTLTJRZMFANCNFSM43AC3EAA>
.
--
THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
|
Beta Was this translation helpful? Give feedback.
-
I can verify that fragments cannot currently call other fragments. That is obviously too severe a limitation to be acceptable, so we'll have to sort it out. |
Beta Was this translation helpful? Give feedback.
-
Fixed in this PR, fragments will be able to call fragments when this lands: |
Beta Was this translation helpful? Give feedback.
-
(As for converting your custom tags to async, yes that will always break
macros, so you should use fragments.
With the fix I just made, fragments should support everything that macros
do, except for the "call" syntax. You are welcome to help implement that
one:
#2930
…On Tue, Apr 20, 2021 at 7:36 AM Miro Yovchev ***@***.***> wrote:
This should make things less painful.
Is the second restriction (render macro inside of fragment, the button
example) also a bug or it's the expected behavior?
I'll wait till your fix is up and I'll update the test case repo so that
is clear what fragment could do.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2914 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAH27KEDLFWTXBGOANKXTTTJVRMJANCNFSM43AC3EAA>
.
--
THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
|
Beta Was this translation helpful? Give feedback.
-
The problem
I had a discussion with the core team about the troubles I'm facing with brand new Apostrophe 3 project, related to newly introduced in the new apos version nunjucks async extensions. I've spent a lot of time to dig and find out what goes wrong (templates failing to render in a very weird ways) or possibly could go wrong in the future in my journey with the the async nunjucks components.
The tests
As a result I've created a repository, based on the latest A3 boilerplate, enhanced with a large number of visually tested code samples. You just need to clone the repo and boot the project on the development machine. Everything is documented (code) and explained (starting from the home page).
I plan to keep this repository around, to expand it and test future versions/related features of Apostrophe.
The summary
If you don't want to dig that deep, here is the summary.
CallExtension
and notCallExtensionAsync
.customTags
as shown in the test project) and they have body (closing tag(s)), be sure to await the body block(s) as it is done here. I think this deserves a special place in the core documentation.Cheers.
Beta Was this translation helpful? Give feedback.
All reactions