search

jstransformers / jstransformer-nunjucks

Nunjucks support for JSTransformers.
http://npm.im/jstransformer-nunjucks
MIT License
6 stars 6 forks source link

support async rendering #35

Open graingert opened 5 years ago

graingert commented 5 years ago

https://mozilla.github.io/nunjucks/api.html#asynchronous-support

ForbesLindesay commented 5 years ago

Looks good. Would you be up for creating a pull request for this?

graingert commented 5 years ago

Seems quite messy to do as the API for, compile -> renderAsync doesn't exist

On Fri, 24 May 2019, 14:51 Forbes Lindesay, notifications@github.com wrote:

Looks good. Would you be up for creating a pull request for this?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jstransformers/jstransformer-nunjucks/issues/35?email_source=notifications&email_token=AADFATG5MIBKTQ3UI76RVGLPW7XFXA5CNFSM4HPOOKL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWFNKMQ#issuecomment-495637810, or mute the thread https://github.com/notifications/unsubscribe-auth/AADFATEB3DEW3INOVPVZ7VLPW7XFXANCNFSM4HPOOKLQ .

ForbesLindesay commented 5 years ago

You could implement the renderAsync method: https://github.com/jstransformers/jstransformer#renderasync

graingert commented 5 years ago

That's not allowed as a result of compile

ForbesLindesay commented 5 years ago

No, I mean you can have a single function renderAsync that does both compile and render in one go. e.g. https://github.com/jstransformers/jstransformer-rollup only implements renderFileAsync.

graingert commented 5 years ago

Hmm, this removes any benefit that users might get from precompiling their templates though

On Sat, 25 May 2019, 01:36 Forbes Lindesay, notifications@github.com wrote:

No, I mean you can have a single function renderAsync that does both compile and render in one go. e.g. https://github.com/jstransformers/jstransformer-rollup only implements renderFileAsync.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jstransformers/jstransformer-nunjucks/issues/35?email_source=notifications&email_token=AADFATDFEQZLAXLETU5AGHLPXCCZ7A5CNFSM4HPOOKL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWGZ22I#issuecomment-495820137, or mute the thread https://github.com/notifications/unsubscribe-auth/AADFATF7GJYKAY7MUAHCMDTPXCCZ7ANCNFSM4HPOOKLQ .

gijswijs commented 4 years ago

I think I have a solution for this, but it snowballed into having to make another PR for test-jstranformer. https://github.com/jstransformers/test-jstransformer/pull/50

I implemented compileAsync. This is enough (I think) to offer the asynchronous support that nunjucks offers. At least it works in my project. But it doesn't pass the tests. This is (again, I think) because the test is wrong. The test assumes that template.fn(locals) returns a string. Hence the test fails when that functions returns a promise, because template.fn(locals).trim() returns an error. By wrapping it in a Promise.resolve() both the situations where template.fn(locals) returns a string or a Promise are covered. This is also how the jstranformer library works, so the tests now better mimick the behavior of the actual library. The test-jstransformer PR also adds a switch for async testing.

If that PR gets accepted, we can update to the latest test-jstransformer release in this repo, and then this PR #42 should pass testing.