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: always return promise if async #2728

Merged
merged 4 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/USING_PRO.md
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ smartypants('"this ... string"')

<h2 id="walk-tokens">Walk Tokens : <code>walkTokens</code></h2>

The walkTokens function gets called with every token. Child tokens are called before moving on to sibling tokens. Each token is passed by reference so updates are persisted when passed to the parser. The return value of the function is ignored.
The walkTokens function gets called with every token. Child tokens are called before moving on to sibling tokens. Each token is passed by reference so updates are persisted when passed to the parser. When [`async`](#async) mode is enabled, the return value is awaited. Otherwise the return value is ignored.

`marked.use()` can be called multiple times with different `walkTokens` functions. Each function will be called in order, starting with the function that was assigned *last*.

Expand Down
28 changes: 19 additions & 9 deletions src/marked.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,28 +108,38 @@ export function marked(src, opt, callback) {
function onError(e) {
e.message += '\nPlease report this to https://github.com/markedjs/marked.';
if (opt.silent) {
return '<p>An error occurred:</p><pre>'
const msg = '<p>An error occurred:</p><pre>'
+ escape(e.message + '', true)
+ '</pre>';
if (opt.async) {
return Promise.resolve(msg);
}
Copy link
Member

Choose a reason for hiding this comment

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

It’s unusual to reject with a string. Usually it’s an Error

Copy link
Member Author

Choose a reason for hiding this comment

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

This is in silent mode so maybe we should actually resolve with the string?

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently the string is returned in silent mode instead of thrown

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated it to resolve on silent and reject with error when not in silent mode. This seems to be the closest async equivalent to what happens when synchronous.

return msg;
}
if (opt.async) {
return Promise.reject(e);
}
throw e;
}

try {
if (opt.async) {
let promise = Promise.resolve(Lexer.lex(src, opt));
if (opt.walkTokens) {
promise = promise.then((tokens) =>
Promise.all(marked.walkTokens(tokens, opt.walkTokens)).then(() => tokens)
);
}
return promise.then((tokens) => Parser.parse(tokens, opt)).catch(onError);
}

const tokens = Lexer.lex(src, opt);
if (opt.walkTokens) {
if (opt.async) {
return Promise.all(marked.walkTokens(tokens, opt.walkTokens))
.then(() => {
return Parser.parse(tokens, opt);
})
.catch(onError);
}
marked.walkTokens(tokens, opt.walkTokens);
}
return Parser.parse(tokens, opt);
} catch (e) {
onError(e);
return onError(e);
}
}

Expand Down
10 changes: 10 additions & 0 deletions test/unit/marked-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1112,4 +1112,14 @@ br
const html = await promise;
expect(html.trim()).toBe('<p><em>text walked</em></p>');
});

it('should return promise if async', async() => {
marked.use({
async: true
});
const promise = marked('*text*');
expect(promise).toBeInstanceOf(Promise);
const html = await promise;
expect(html.trim()).toBe('<p><em>text</em></p>');
});
});