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

feat: add support for async compiling #42

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gijswijs
Copy link

jstransformer-nunjucks doesn't support asynchronous rendering, but nunjucks does, as explained in issue #35.

The basic jstransformer library looks for compileAsync, just before it defaults to the compile method as a final resort when compileFileAsync is called. This PR implement compileAsync

Nunjucks uses callbacks for asynchronous support, but jstransformer expects promises. Therefor the render function is denodified, which means that the render functions is wrapped in a promise, and the callback resolves the promise.

@ci-reporter
Copy link

ci-reporter bot commented May 27, 2020

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Travis CI build is failing as of c58982a. Here's the output:

npm test
> [email protected] test /home/travis/build/jstransformers/jstransformer-nunjucks
> test-jstransformer

 • nunjucks
   ✓ transform has an output format (0ms)
   ✓ transform has input formats (0ms)
   • basic
     ✓ nunjucks.compile() (5ms)
     ✗ nunjucks.compileAsync() (1ms)

       TypeError: template(...).trim is not a function
           at checkFunctionOutput (/home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/test-jstransformer/index.js:88:38)
           at transform.compileAsync.then.template (/home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/test-jstransformer/index.js:117:11)
           at tryCallOne (/home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/promise/lib/core.js:37:12)
           at /home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/promise/lib/core.js:123:15
           at flush (/home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/asap/raw.js:50:29)
           at _combinedTickCallback (internal/process/next_tick.js:132:7)
           at process._tickCallback (internal/process/next_tick.js:181:9)

Total duration 15ms
tests failed

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!


Failed build for d427f8f
npm test
> [email protected] test /home/travis/build/jstransformers/jstransformer-nunjucks
> test-jstransformer

 • nunjucks
   ✓ transform has an output format (0ms)
   ✓ transform has input formats (0ms)
   • basic
     ✓ nunjucks.compile() (6ms)
     ✗ nunjucks.compileAsync() (1ms)

       TypeError: transform.compileAsync(...).then is not a function
           at TestCase.test [as fn] (/home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/test-jstransformer/index.js:116:55)
           at /home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/testit/lib/suite.js:74:29
           at tryCallOne (/home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/promise/lib/core.js:37:12)
           at /home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/promise/lib/core.js:123:15
           at flush (/home/travis/build/jstransformers/jstransformer-nunjucks/node_modules/asap/raw.js:50:29)
           at _combinedTickCallback (internal/process/next_tick.js:132:7)
           at process._tickCallback (internal/process/next_tick.js:181:9)

Total duration 15ms
tests failed

This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

@gijswijs
Copy link
Author

I think the tests are 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.

I also made a PR for the tests: jstransformers/test-jstransformer#50
If that one gets merged, we can go ahead and see if this passes.

@gijswijs gijswijs changed the title Add support for async compiling (and rendering) feat: add support for async compiling May 28, 2020
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

Successfully merging this pull request may close these issues.

1 participant