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
Dart2JS should use Promise.then for scheduleImmediate #20055
Comments
Dart2JS* still uses a bunch of hacks to get into the native micro-task queue: @patch
class _AsyncRun {
@patch
static void _scheduleImmediate(void callback()) {
_scheduleImmediateClosure(callback);
}
// Lazily initialized.
static final Function _scheduleImmediateClosure =
_initializeScheduleImmediate();
static Function _initializeScheduleImmediate() {
requiresPreamble();
if (JS('', 'self.scheduleImmediate') != null) {
return _scheduleImmediateJsOverride;
}
if (JS('', 'self.MutationObserver') != null &&
JS('', 'self.document') != null) {
// Use mutationObservers.
var div = JS('', 'self.document.createElement("div")');
var span = JS('', 'self.document.createElement("span")');
var storedCallback;
internalCallback(_) {
var f = storedCallback;
storedCallback = null;
f();
}
var observer = JS('', 'new self.MutationObserver(#)',
convertDartClosureToJS(internalCallback, 1));
JS('', '#.observe(#, { childList: true })', observer, div);
return (void callback()) {
assert(storedCallback == null);
storedCallback = callback;
// Because of a broken shadow-dom polyfill we have to change the
// children instead a cheap property.
// See https://github.com/Polymer/ShadowDOM/issues/468
JS('', '#.firstChild ? #.removeChild(#): #.appendChild(#)', div, div,
span, div, span);
};
} else if (JS('', 'self.setImmediate') != null) {
return _scheduleImmediateWithSetImmediate;
}
// TODO(20055): We should use DOM promises when available.
return _scheduleImmediateWithTimer;
}
static void _scheduleImmediateJsOverride(void callback()) {
internalCallback() {
callback();
}
JS('void', 'self.scheduleImmediate(#)',
convertDartClosureToJS(internalCallback, 0));
}
static void _scheduleImmediateWithSetImmediate(void callback()) {
internalCallback() {
callback();
}
JS('void', 'self.setImmediate(#)',
convertDartClosureToJS(internalCallback, 0));
}
static void _scheduleImmediateWithTimer(void callback()) {
Timer._createTimer(Duration.zero, callback);
}
} As of modern browsers, this can be reduced to a single function: @patch
class _AsyncRun {
@patch
static void _scheduleImmediate(void Function() callback) {
JS('void', 'Promise.prototype.then(#)', convertDartClosureToJS(callback, 0));
}
} *(and DDC, I'm not sure? @jmesserly) |
dart2js still supports IE11, I believe. for DDC, we can simplify it now. |
I mean, if under some flag Dart2JS wants to emit the old code, they are welcome. Our biggest and most demanding clients don't need the overhead of running IE11 compatible code. The other option is we could write our own Dart2JS "rewriter" layer internally, but I don't think that's the best option here. |
we continue to support IE11 and we are starting a plan to be able to generate better code when IE11 is not needed. However, our plan requires a lot preparation on the compiler and testing infrastructure. Happy to share more details if you are interested, but I don't see us switching to Promise.then for a few months until we are ready. |
We can polyfill this case. More details when I return.
…On Fri, Nov 2, 2018, 11:19 sigmundch ***@***.*** wrote:
we continue to support IE11 and we are starting a plan to be able to
generate better code when IE11 is not needed. However, our plan requires a
lot preparation on the compiler and testing infrastructure. Happy to share
more details if you are interested, but I don't see us switching to
Promise.then for a few months until we are ready.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20055 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACAtlZKQOjcif51W0AlEy8dGl3RIGRThks5urIykgaJpZM4YKDe6>
.
|
@Markzipan just discovered, this issue causes inconsistent timing with |
testing a CL now for DDC (https://dart-review.googlesource.com/c/sdk/+/91765) |
about to send out the CL. One interesting thing I found: this does cause unhandled exceptions to be treated as unhandled Promise rejects. In browsers, this means the error goes to the 'unhandledrejection' event rather than the 'error' event, so node.js is going to cause unhandled promise rejections to exit the process: https://medium.com/@dtinth/making-unhandled-promise-rejections-crash-the-node-js-process-ffc27cfcc9dd ... for now, it prints a warning. Dart VM behavior is to exit, so it'll be consistent. edit: also I need to land https://dart-review.googlesource.com/c/sdk/+/92247 before my CL above. |
All of DDC's supported platforms have Promises, so we can use them instead of MutationObservers (web) and timers (node.js). See issue #20055 (same issue, but for dart2js). Change-Id: Id635a4a9fa104a2ab19dd20824d209f682f831f9 Reviewed-on: https://dart-review.googlesource.com/c/91765 Reviewed-by: Mark Zhou <markzipan@google.com> Commit-Queue: Jenny Messerly <jmesserly@google.com>
Dart2js should use DOM Promises when available to scheduleImmediate.
Currently FF is broken and doesn't allow us to switch.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1038631
The text was updated successfully, but these errors were encountered: