Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add TypeScript type define and Promise support.#99

Closed
septs wants to merge 4 commits into
abbr:master from
NiceLabs:master
Closed

Add TypeScript type define and Promise support. #99
septs wants to merge 4 commits into
abbr:master from
NiceLabs:master

Conversation

@septs

@septs septs commented Aug 5, 2018

Copy link
Copy Markdown

No description provided.

@septs septs changed the title (削除) Add TypeScript type defines. (削除ここまで) (追記) Add TypeScript type define and Promise support. (追記ここまで) Aug 5, 2018

septs commented Aug 5, 2018
edited
Loading

Copy link
Copy Markdown
Author

#96 #90 type information provided is wrong.

#90 deasync.await implements an unhandled reject.
https://github.com/abbr/deasync/pull/90/files#diff-168726dbe96b3ce427e7fedce31bb0bcR77

septs commented Aug 12, 2018

Copy link
Copy Markdown
Author

abbr commented Aug 13, 2018

Copy link
Copy Markdown
Owner

Thanks for the pr. I am ok for the ts but reluctant to add promise and await because they conceptually conflict with deasync. What's the use case that requires deasync support promise?

septs commented Aug 13, 2018

Copy link
Copy Markdown
Author

Simultaneously provide users with asynchronous and synchronous.

async function execCommand() { /* ...code... */ }
function execCommandSync() { return deasync.await(execCommand()) }

septs commented Aug 15, 2018
edited
Loading

Copy link
Copy Markdown
Author

If you really don't want to accept Promise support. I will remove the Promise support. @abbr

septs commented Sep 29, 2018

Copy link
Copy Markdown
Author

Comment thread index.d.ts
@septs septs closed this Jan 27, 2019
Comment thread index.js
Comment on lines +79 to +89
var resolved = false, rejected = false,
result, error;
promise.then(
function (value) { resolved = true; result = value; return result; },
function (reason) { rejected = true; error = reason; return reason; }
)
module.exports.loopWhile(function () { return !resolved && !rejected; })
if (rejected) {
throw error;
}
return result;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not work with async terser.minify()
the promise is never resolved or rejected = deadloop

return result and return reason in the callback fns is pointless

dhorkin commented Nov 10, 2023
edited
Loading

Copy link
Copy Markdown

Thanks for the pr. I am ok for the ts but reluctant to add promise and await because they conceptually conflict with deasync. What's the use case that requires deasync support promise?

In what way do they "conceptually conflict" ? The whole point of deasync is to be able to run async code synchronously, isn't it?

For example, suppose you have the following and you need to have to run it in a synchronous context:

async function foo() {
 return true;
}

My expectation was I'd be able to then do this in a place where I cannot make a method asynchronous in order to use await (and an IIFE is not a viable solution):

const syncFoo = deasync(foo);
let result = syncFoo();
somethingElse(result); // won't run until the promise from foo resolves

But this gives various type errors. I haven't actually tried running, but presumably if TypeScript is complaining about it, then deasync as a library can't currently do the one use case I actually need it for.

This is shocking to me, given the library is described as DeAsync turns async function into sync. Async functions by definition return promises... The example with child_process.exec might be asynchronous process creation, but it is not an async function; it immediately returns the ChildProcess object!

milahu commented Nov 10, 2023

Copy link
Copy Markdown

I haven't actually tried running

just try, for science ; )

alternatives to deasync: terser/terser#801 (comment)

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

Reviewers

@abbr abbr abbr left review comments
+1 more reviewer
@milahu milahu milahu left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /