-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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: Preprocessors may return promises, but not resolve to 'content'. #3383
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
✅ Build karma 2426 completed (commit 9d6e4cf686 by @seebees) |
✅ Build karma 27 completed (commit 9d6e4cf686 by @seebees) |
✅ Build karma 28 completed (commit 9d6e4cf686 by @seebees) |
The original API for Preprocessors assumed callback. Now promises are a valid return. If an implementer returns a promise from `process` but only resolves content via the callback, then while the result of `process` is a promise, it will not resolve to the intended content. An easy example is for `process` to be an `async` function. e.g. ``` async function process(content, file, done) { const newContent = await magic(content) done(newContent) } ```
4620550
to
59ec115
Compare
✅ Build karma 2428 completed (commit cc81357af3 by @seebees) |
✅ Build karma 30 completed (commit cc81357af3 by @seebees) |
✅ Build karma 29 completed (commit cc81357af3 by @seebees) |
A link to the "breaking" PR: #3376 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding this issue. The author of the async change has a similar fix in PR #3387 with unit tests for both paths.
even better! I would add a comment since this is a little edgy, but that is a style choice :) |
4.4.1 is up |
The original API for Preprocessors assumed callback.
Now promises are a valid return.
If an implementer returns a promise from
process
but only resolves content via the callback,
then while the result of
process
is a promise,it will not resolve to the intended content.
An easy example is for
process
to be anasync
function.e.g.