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

Support asynchronous filters #322

Open
Awk34 opened this issue Jan 3, 2017 · 8 comments
Open

Support asynchronous filters #322

Awk34 opened this issue Jan 3, 2017 · 8 comments
Assignees

Comments

@Awk34
Copy link

Awk34 commented Jan 3, 2017

My issue is that I'm trying to use mathjax-node to create a showdown extension, but mathjax only exposes an asynchronous API. Ex:

var mjAPI = require("mathjax-node/lib/mj-single.js");
mjAPI.start();

var yourMath = 'E = mc^2';

mjAPI.typeset({
  math: yourMath,
  format: "TeX", // "inline-TeX", "MathML"
  svg: true,
}, data => {
  if (data.errors) return data.errors;

  console.log(data.svg);
});

Would it be difficult to allow for the filter function in an extension to accept a promise that resolves to a string?

@Awk34
Copy link
Author

Awk34 commented Jan 3, 2017

It looks like it would be decently difficult, seeing as how all calls to makehtml assume it synchronously returns the text. Something like this would probably affect a lot of files.

@tivie tivie self-assigned this Jan 4, 2017
@tivie
Copy link
Member

tivie commented Jan 4, 2017

Due to the nature of markdown itself, each block, to be parsed correctly, requires context.
For instance,

    - bar

can either mean:

  • a sub list
    - foo
        - bar
    
  • or a block of code

This means the parsing, to catch all possible edge cases, must be done sequentially.
Turning showdown in async would prove extremely dificult.

However, I think you can pre parse the text with math-jax and then parse it through showdown. Since html is valid in markdown, showdown would leave those blocks alone.

@tivie tivie added the question label Jan 4, 2017
@Awk34
Copy link
Author

Awk34 commented Jan 4, 2017

This is the solution that I ended up on: https://github.com/Awk34/awk34_project_euler/blob/9bd20689c98be289d66112ac48d0d81404ce93ea/js/util/markdownTools.js

I'm not saying all of the parsing has to be asynchronous, and to be done non-sequentially. It would just be nice if a subParser could handle an asynchronous response. That way I could use the callback response of MathJax in an extension. The full converter could still wait for the subParser to finish, but it would be done with an asynchronous callback/promise instead of assuming that the subParser returns text in a synchronous matter. Like this:

let text = '...';

text = syncParser1(text);

asyncParser(text).then(newText => {
  text = newText;

  text = syncParser2(text);

  return text;
});

@tivie
Copy link
Member

tivie commented Jan 5, 2017

@Awk34 I think I might be missunderstanding you (or failing to grasp the concept), but I don't see how it can be accomplished properly.

I mean, sure, I can wrap every subparser in a promise, since you can "inject" an extension before or after any subparser, but it seems you had a simpler way in mind...

@Awk34
Copy link
Author

Awk34 commented Jan 9, 2017

I think this is a better example:

// settable extensions
let extensions = [ext1, ext2, ...];

async function convert(text) {
  // Showdown converters
  text = syncParser1(text);

  // User extensions
  for(let i = 0; i < extensions.length; i++) {
    let result = extensions[i](text);

    // if extension returns a promise, wait for it
    // if not, assume synchronous extension returning mutated text
    if(result instanceof Promise) {
      text = await result;
    } else {
      text = result;
    }
  }

  // more Showdown converters
  text = syncParser2(text);

  return text;
}

Assume extensions is an array of extension functions, and assume convert is the main Showdown converter function. Instead of assuming that all extensions synchronously return text, it allows for them to also return a Promise, in which it waits for the promise to be fulfilled, and then moves onto the next extension. I hope this gets my point across better.

@tivie
Copy link
Member

tivie commented Jan 10, 2017

Since parser order is mandatory, and they have to run sequentially, in your example the text = syncParser2(text); can only run after the extension completes execution or the extension might not work.

So, I can only think of 2 ways of allowing async extensions properly:

  1. block the execution of the parser and wait for said extensions to finish processing (effectively turning async into sync)
  2. Rewriting the library, making each sub-parser async,

Scenario 1

async / await browser support is currently very limited and showdown should be reasonable cross browser compatible (ES5).
With ECMAScript 5, I don't know how to block execution and wait for promises to resolve.
@Awk34 Maybe there is a way?

Scenario 2

This is possible, obviously,
But it would require a complete refactoring of the library and respective tests, something I would rather not do.

@Awk34 I was hoping I've missed something, and there was a 3rd option.

@Awk34
Copy link
Author

Awk34 commented Jan 10, 2017

I wrote my example with async/await just for clarity's sake. You can see how Babel would convert this to ES5 here. The Promise check would probably have to be changed to something like if(typepof result.then === 'function')

@tivie
Copy link
Member

tivie commented Jan 20, 2017

I will put this on pending. Although I think I found a way to implement this, this requires a bit of rewriting in the library and an overhaul of the testing framework too, to support async tests.

@Awk34 I welcome any PR though, so if you're interested!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants