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

expose a way to use a custom/external microTask impl #439

Closed
maxhq opened this issue Jan 22, 2021 · 5 comments
Closed

expose a way to use a custom/external microTask impl #439

maxhq opened this issue Jan 22, 2021 · 5 comments

Comments

@maxhq
Copy link

maxhq commented Jan 22, 2021

The use of a Promise for microTask() prevents user code from catching any errors that might occur in the function passed to microTask() - i.e. commit().
This happens e.g. if the data Array contains less entries than what opts.series define.

The problem can easily be shown by this code:

// User code with Promise.catch()
new Promise((resolve, reject) => {
  // microTask()
  Promise.resolve().then(
    // code passed to microTask() throws some error:
    () => { throw new Error("Test error") }
  )
})
.catch(e => console.error("Caught via Promise.catch()", e)) // not called

// User code with try-catch
try {
  // microTask()
  Promise.resolve().then(
    // code passed to microTask() throws some error:
    () => { throw new Error("Test error") }
  )
}
catch(e) { console.error("Caught via try-catch", e) } // not called

I cannot think of an easy solution other than turning uPlot itself into a Promise. But it's late and I'm tired and there might be an easy solution :)

@maxhq
Copy link
Author

maxhq commented Jan 22, 2021

Just for the record: this is a fantastic piece of software and in my 20+ years as a developer I have rarely seen modules as efficient, pure, well thought of and fun to use as uPlot. Thank you!

@leeoniya
Copy link
Owner

Just for the record: this is a fantastic piece of software and in my 20+ years as a developer I have rarely seen modules as efficient, pure, well thought of and fun to use as uPlot. Thank you!

thanks! glad you're enjoying it :)

I cannot think of an easy solution other than turning uPlot itself into a Promise. But it's late and I'm tired and there might be an easy solution :)

unfortunately, i'm far from an event loop expert. i thought maybe this might help, but i don't think it does: feross/queue-microtask#2 (comment)

@maxhq
Copy link
Author

maxhq commented Jan 29, 2021

unfortunately, i'm far from an event loop expert. i thought maybe this might help, but i don't think it does: feross/queue-microtask#2 (comment)

I tried it but you are right, it doesn't help: obviously Window.queueMicrotask() is already defined so I cannot alter it.

What whould be really helpful is if you allowed for configuration of microTask() e.g. via options so that commit() (https://github.com/leeoniya/uPlot/blob/master/src/uPlot.js#L1591) would use a custom function.

@leeoniya
Copy link
Owner

What whould be really helpful is if you allowed for configuration of microTask() e.g. via options

💯 , feel free to PR this. (plz leave the builds out of the PR for easier review, i'll build post-merge)

@leeoniya leeoniya changed the title Use of Promise for microTask() prevents catching errors expose a way to use a custom/external microTask impl Jan 29, 2021
@leeoniya
Copy link
Owner

hey @maxhq

if you still need this, please open a PR. i'm gonna close this out for now as i don't plan to add it since this is the only request i've encountered since switching to async batching.

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

2 participants