search

ghcjs / ghcjs-base

base library for GHCJS for JavaScript interaction and marshalling, used by higher level libraries like JSC
MIT License
45 stars 67 forks source link

syncCallback' and its siblings should accept `OnBlocked`. #82

Closed crocket closed 7 years ago

crocket commented 7 years ago

If a callback function passed to syncCallback' executes putStrLn multiple times, the callback throws an exception.

This is not desirable. It should continue asynchronously by default, or it should accept OnBlocked.

crocket commented 7 years ago

Does ThrowWouldBlock make sense in any case?

hamishmack commented 7 years ago

What would it return to JavaScript if it did not throw an exception?

crocket commented 7 years ago

Is it impossible to return anything from an asynchronous callback?

hamishmack commented 7 years ago

In the case of syncCallback the caller will be expecting a value (otherwise they could have just used asyncCallback). So we will need to consider what value we return when the callback has not yet finished. One option might be to pass "on blocking result" JSVal to syncCallback (then the user can choose).

hamishmack commented 7 years ago

Is it impossible to return anything from an asynchronous callback?

Yes.

crocket commented 7 years ago

Yes.

It seems syncCallback' and its siblings are not useful because they cannot be used with side effects. Side effects can block a thread. I think I can document this issue on README.

In the case of syncCallback the caller will be expecting a value

syncCallback :: OnBlocked -> IO () -> IO (Callback (IO ())) doesn't seem to return a value to javascript.

One option might be to pass "on blocking result" JSVal to syncCallback (then the user can choose).

What do you mean?

hamishmack commented 7 years ago

Sorry wrote those responses in a rush.

It seems syncCallback' and its siblings are not useful because it cannot be used with side effects. Side effects can block a thread. I think I can document this issue on README.

I think you realise, but not all functions with side effects can block a thread. putStrLn can because it simulates the buffering and tries to avoid blocking all the other things that JavaScript might be doing.

Having said that syncCallback' should be avoided and syncCallback is probably the best option in most cases and it would be good to reflect that in the documentation.

In the case of syncCallback the caller will be expecting a value

syncCallback :: OnBlocked -> IO () -> IO (Callback (IO ())) doesn't seem to return a value to javascript.

Sorry I meant to say:

In the case of syncCallback' the caller will be expecting a value (otherwise they could have just used syncCallback).

One option might be to pass "on blocking result" JSVal to syncCallback (then the user can choose).

What do you mean?

Again I was thinking of syncCallback' but got the names mixed up.

When the JavaScript callback is called it has to return something (even if it returns undefined). Currently I think syncCallback returns undefined then it is asked to ContinueAsync. This is unsurprising to the caller because they were never expecting to get anything back anyway.

syncCallback' would also have to return something if it was going to ContinueAsync. However in this case the caller will be expecting something to be returned. Instead of returning undefined it might be nicer to let the creator of the callback specify what the return value should be if the callback it going to ContinueAsync.

The function might be something like

syncCallbackContinueAync :: JSVal -> IO JSVal -> IO (Callback (IO JSVal))

That extra JSVal argument would be the default value returned to the JavaScript caller when the callback it going to be continued asynchronously. The real return value would have to be discarded in that situation (because it would be computed too late to be returned).

I think we really need some example use cases though so that we can figure out what (if anything) will be useful.

I have never used syncCallback' and ghcjs-dom does not create wrappers for it (it creates type safe wrappers for syncCallback ThrowWouldBlock, syncCallback ContinueAsync and asyncCallback).

crocket commented 7 years ago

If syncCallback' and its derivatives weren't used, then we wouldn't have to solve this problem. Thus, I close this issue. When it becomes an actual problem, an issue will be opened.