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

async: true inconsistent return type (Promise | string) + docs update #2721

Closed
webketje opened this issue Jan 26, 2023 · 2 comments · Fixed by #2728
Closed

async: true inconsistent return type (Promise | string) + docs update #2721

webketje opened this issue Jan 26, 2023 · 2 comments · Fixed by #2728
Labels
L2 - annoying Similar to L1 - broken but there is a known workaround available for the issue

Comments

@webketje
Copy link

webketje commented Jan 26, 2023

What pain point are you perceiving?.
marked provides an async option to enable asynchronous walkTokens.
@types/marked for version 4.x provides the following overload which looks perfectly logical/ reasonable:

export function marked(src: string, options: marked.MarkedOptions & {async: true}): Promise<string>;

However, the async true option only returns a promise if there is an async walkTokens.
Compare:

const immediatelyRendered = marked('# h1', { async: true })
// returns  '<h1>h1</h1>'

const notRerendered = marked('# h1', {
  async: true,
  walkTokens(token) {
    setTimeout(() => { token += 'h' }, 1000)
  }
})
// returns  '<h1>h1</h1>'

const soonRendered = marked('# h1', {
  async: true,
  walkTokens() {
    return Promise.resolve()
  }
})
// returns Promise<'<h1>h1</h1>'>

Additionally, in the docs it is said under the Walk Tokens section:

The return value of the function is ignored.

Evidently, this is not (or no longer) true

Describe the solution you'd like
When a user instructs marked to render async: true the API should consistently return a Promise, regardless of whether there is an async walkTokens extension included. Additionally, the docs should be updated to "The return value should be a promise for asynchronous walkTokens functions.

Tested using the CJS build of marked 4.2.12

@UziTech
Copy link
Member

UziTech commented Jan 29, 2023

Nice catch. If you want to create a PR with those changes that would be much appreciated 😁👍

@UziTech UziTech added the L2 - annoying Similar to L1 - broken but there is a known workaround available for the issue label Jan 29, 2023
@UziTech
Copy link
Member

UziTech commented Feb 4, 2023

I created #2728 which should fix both of these issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L2 - annoying Similar to L1 - broken but there is a known workaround available for the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants