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

Switch to a Promise interface #76

Closed
sindresorhus opened this issue Jul 31, 2016 · 8 comments
Closed

Switch to a Promise interface #76

sindresorhus opened this issue Jul 31, 2016 · 8 comments
Labels
🙋 no/question This does not need any changes

Comments

@sindresorhus
Copy link

For the async API. Instead of a callback.

@wooorm
Copy link
Member

wooorm commented Jul 31, 2016

For the rules/externals? Yup, why not! Shouldn’t be very hard!

@sindresorhus
Copy link
Author

Pretty much for everything, but was mainly thinking of the callback here: var file = remark().use(lint).process('## Hello world!', () => {});

@wooorm
Copy link
Member

wooorm commented Aug 2, 2016

With the release of the master branch, in ± a few minutes, it’ll be possible to return promises from rules. It’s already possible to return promises from remark plug-ins too.

However, I personally don’t want to support promises in the outward remark API, so that (.process(doc, cb)) will remain a callback!

@wooorm wooorm closed this as completed Aug 2, 2016
@sindresorhus
Copy link
Author

With the release of the master branch, in ± a few minutes, it’ll be possible to return promises from rules. It’s already possible to return promises from remark plug-ins too.

Awesome! Thanks for being so responsive :)

However, I personally don’t want to support promises in the outward remark API, so that (.process(doc, cb)) will remain a callback!

You don't have to switch though. You could easily support both. Return a promise if no callback is provided. That's a very common thing to do for authors that are not convinced about promises, but want to make it easy for anyone to use their API.

@wooorm
Copy link
Member

wooorm commented Aug 4, 2016

Unfortunately not: the vfile itself is already returned, which is useful, as the API is often sync.

If that’d be possible I’d be OK with it, but I value the sync API too much!

@sindresorhus
Copy link
Author

Having an API be sync or async depending an argument is generally considered an anti-pattern. Would be better to be explicit about with through an option or even better a separate method.

  • .run()
  • .runSync()

Or the inverse.

@sindresorhus
Copy link
Author

If you want to preserve backwards compatibility, you could add a .runAsync() method.

@wooorm
Copy link
Member

wooorm commented Aug 6, 2016

Currently it’s more that the plugins someone uses defines the process. You’ve convinced me though: superseded by unifiedjs/unified#8!

@wooorm wooorm added the 🙋 no/question This does not need any changes label Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 no/question This does not need any changes
Development

No branches or pull requests

2 participants