m.request not redrawing automatically in Chrome #2426

sagebind · 2019-06-07T22:25:17Z

Mithril Version: 1.1.6

When using m.request inside an oninit lifecycle method that returns a promise, Mithril does not redraw automatically in Chrome like it should when the request finishes and the promise resolves. It does work in Firefox and Safari.

Chrome version: 75.0.3770.80
Firefox version: 68.0b6
Safari version: 12.1.1 (14607.2.6.1.1)

Expected Behavior

Mithril should redraw automatically when an Ajax request finishes.
Mithril should behave consistently between Chrome and Firefox at the very least.

Current Behavior

The redraw callback is never called in Chrome.

Possible Solution

I tried stepping through Mithril in the browser debugger on Chrome and Firefox to figure out what is different between the two, and in Chrome it seems like this callback is never getting invoked: https://github.com/MithrilJS/mithril.js/blob/v1.1.6/request/request.js#L20-L26. Maybe Chrome does not allow reassigning then on a native Promise object? If that is the case, maybe there's some other way of doing this, like creating a new promise and wrapping the old one.

As a workaround, I am calling m.redraw() manually at the end of my oninit lifecycle in my actual application (company Intranet project).

Steps to Reproduce

Here is an example tiny app that reproduces the problem:

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="utf-8">
        <title>Demo</title>
    </head>
    <body>
        <script src="https://unpkg.com/[email protected]/mithril.js"></script>
        <script>
            class Page {
                async oninit(vnode) {
                    let response = await m.request("https://reqres.in/api/users/2");
                    this.user = response.data;
                }

                view(vnode) {
                    return m("main", [
                        m("h1", "Demo App"),
                        this.user ? m(User, {
                            user: this.user,
                        }) : "Loading..."
                    ]);
                }
            }

            class User {
                view(vnode) {
                    return [
                        m("p", `Name: ${vnode.attrs.user.first_name} ${vnode.attrs.user.last_name}`),
                        m("p", `Email: ${vnode.attrs.user.email}`),
                    ];
                }
            }

            m.mount(document.body, Page);
        </script>
    </body>
</html>

In Firefox, this shows Loading... for a short time and then displays user information. On Chrome, this displays Loading... forever unless you call m.redraw() manually in the JavaScript console.

Context

Additional Information

Your Environment

Browser Name and version: Chrome 75.0.3770.80
Operating System and version (desktop or mobile): macOS Mojave 10.14.5
Link to your project:

The text was updated successfully, but these errors were encountered:

spacejack · 2019-06-08T02:09:15Z

You might try rc6 (npm i [email protected]) to see if it solves it.

The problem is the redraw occurs after the request promise chain ends. You await m.request() so the redraw happens before this line runs: this.user = response.data;

One solution is to use promises:

oninit ({state}) {
  m.request("https://reqres.in/api/users/2"),then(response => {
    state.user = response.data;
  }) // Redraw happens after this chain resolves.
}

Another is to manually redraw:

async oninit(vnode) {
  let response = await m.request({
    url: "https://reqres.in/api/users/2",
    background: true
  });
  this.user = response.data;
  m.redraw();
}

sagebind · 2019-06-08T02:30:05Z

@spacejack Just tried 2.0.0-rc.6 (just replace 1.1.6 in the example app I included) and the behavior is the same.

A couple of things:

The problem is the redraw occurs after the request promise chain ends.

If that is the case, then why does it work in other browsers, just not Chrome?

One solution is to use promises:

oninit ({state}) {
 m.request("https://reqres.in/api/users/2"),then(response => {
    state.user = response.data;
  }) // Redraw happens after this chain resolves.
}

Well an async function is just syntax sugar (to some extent) for promises, no? My async function should be semantically equivalent to something like this:

oninit(vnode) {
    return m.request("https://reqres.in/api/users/2").then(response => {
        this.user = response.data;
    });
}

await adds to the promise chain in the same way that calling then does. According to MDN, await will literally invoke then with a native callback as an argument.

I decided to verify that indeed Chrome follows this behavior, and it appears that it does. This example works in both Chrome and Firefox in the way you expect:

const test = async () => {
    await { // not a promise, but does have a `then` method
        then(callback) {
            setTimeout(() => callback(), 5000);
        }
    };
    alert("slept!");
};
test();

spacejack · 2019-06-08T02:36:36Z

Sorry, I ran into a similar problem a while ago and maybe my assessment of it was wrong. I don't understand why it works in other browsers either.

What if you try

async oninit(vnode) {
  this.user = (await m.request("https://reqres.in/api/users/2")).data;
}

I was under the impression that this should work in v2:

let response = await m.request("https://reqres.in/api/users/2");
this.user = response.data;

If not I think it would be a bug.

sagebind · 2019-06-08T02:40:58Z

Nope, that behaves the same way. Still works in Firefox, still doesn't work in Chrome. You should be able to open the HTML file I provided in the issue description to reproduce yourself too, if that helps.

Like I said, I did some poking around in the debugger and it seemed like something odd was going on with setting up the finalizer for m.request that didn't happen in other browsers, but I gave up eventually because I wasn't familiar with the code.

spacejack · 2019-06-08T02:59:46Z

Here is a flems repro of your example with Mithril 2.0-rc6. If you swap to the Promise-based oninit it works.

I think the reason that await m.request was supposed to work in V2 is that m.redraw is now async (by way of requestAnimationFrame.) So the await m.request promise chain would resolve before the requestAnimationFrame in m.redraw. Maybe this is not always the case in Chrome.

osban · 2019-06-08T10:06:41Z

You'd expect the code to work like this, but yeah, in Chrome it doesn't.
Another workaround could be to just use another then, like this.

dead-claudia · 2019-06-09T06:45:31Z

First, I'm seeing the same issue on v1.x with Chrome. That repro lets you compare both through a couple simple buttons.

Second, I strongly suspect a V8 bug here. It repros even without the DOM. If it's not a V8 issue, then I suspect it's either a spec bug or a design limitation I can't really work around. I've filed https://bugs.chromium.org/p/v8/issues/detail?id=9349 to track this.

spacejack · 2019-06-09T14:51:27Z

@isiahmeadows was wondering, with redraws being async (by way of RAF) does m.request still need to use its 'finalizer'?

This would break cases where you're relying on it to defer the redraw until another long-running (i.e., longer than RAF) promise is appended to the chain (and you might need to use background: true if you're chaining several requests.) But it might work for the common cases like this one.

dead-claudia · 2019-06-10T03:13:38Z

@spacejack Yes. The event loop in modern browsers obviously can't run animation callbacks while draining the microtask queue, because those are always drained at highest priority as a single step, and as per spec they aren't handled as a normal "task queue" like the task queues for requests and timers are. However, you could schedule a new Promise((resolve) => setTimeout(resolve, 0)), and that could result in an animation frame in the middle, if you call that more than once. It's pretty well-known the minimum delay is 4ms (well, only with a nesting depth of at least 5), but it only takes 4-5 of those to hit a 16ms animation frame, and the Promise polyfill in practice could easily hit not only the nesting depth requirement of 5, but also the remaining 4-5 delays required to hit that nesting depth, because, like most other userland Promises/A+ implementations targeting browsers, it specifically uses setTimeout(callback, 0) to defer and it doesn't try to batch tasks and emulate its own microtask queue. (Bluebird is a rare exception here, and most Node implementations use process.nextTick to correctly schedule microtasks.)

Here's the relevant parts of the spec:

Event loop processing model: https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model
Timer initialization steps: https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timer-initialisation-steps

dead-claudia · 2019-06-10T05:48:30Z

Figured it out: V8 is actually doing the right thing here, but nobody else is and so that's why this shows up in Chrome, but not Firefox or Safari: tc39/ecma262#1577. It's exceedingly subtle, but it comes down to this:

The .then method on the object is only called when wrapping promises, no earlier and no later.
If p.constructor === Promise, the object is not supposed to be wrapped, but instead returned directly. (This gives rise to a genuine spec bug, but is otherwise unrelated to this issue.)

Gotta love spec bugs and implementation divergence. 🙂

The proper workaround is two-part and very subtle:

Add function PromiseProxy(executor) { return new Promise(executor) }; PromiseProxy.prototype = Promise.prototype; PromiseProxy.__proto__ = Promise here to reduce runtime impact. The first is to work with async functions as explained above. The rest is to make the wrapper as transparent as possible, in case the global Promise Mithril sees relies on this.constructor.foo or p instanceof this.constructor for some reason.
Before this line, add the line promise.constructor = PromiseProxy.

spacejack · 2019-06-10T14:18:24Z

Nice work @isiahmeadows.

sagebind · 2019-06-10T19:23:54Z

Good investigation! The solution is way above my head, but still interesting to read.

dead-claudia · 2019-06-10T23:27:32Z

@sagebind If you'd like to know a bit more about the background leading up to my fix, I wrote this Twitter thread on it, targeting a less technical audience that includes non-programmers.

dead-claudia mentioned this issue Jun 10, 2019

Engines don't agree with each other on how to await promise instances with overridden .then properties tc39/ecma262#1577

Open

dead-claudia mentioned this issue Jun 10, 2019

Make m.request work with async/await correctly. #2428

Merged

11 tasks

dead-claudia closed this as completed in #2428 Jun 10, 2019

dead-claudia added this to Triage/bugs Sep 2, 2024

dead-claudia moved this to Closed in Triage/bugs Sep 2, 2024

dead-claudia mentioned this issue Apr 4, 2025

Fix domFor hitting null dom elements in while loop #3013

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

m.request not redrawing automatically in Chrome #2426

m.request not redrawing automatically in Chrome #2426

sagebind commented Jun 7, 2019 •

edited

Loading

Your Environment

spacejack commented Jun 8, 2019 •

edited

Loading

Uh oh!

sagebind commented Jun 8, 2019 •

edited

Loading

Uh oh!

spacejack commented Jun 8, 2019 •

edited

Loading

Uh oh!

sagebind commented Jun 8, 2019 •

edited

Loading

Uh oh!

spacejack commented Jun 8, 2019 •

edited

Loading

Uh oh!

osban commented Jun 8, 2019

Uh oh!

dead-claudia commented Jun 9, 2019

Uh oh!

spacejack commented Jun 9, 2019

Uh oh!

dead-claudia commented Jun 10, 2019

Uh oh!

dead-claudia commented Jun 10, 2019

Uh oh!

spacejack commented Jun 10, 2019

Uh oh!

sagebind commented Jun 10, 2019

Uh oh!

dead-claudia commented Jun 10, 2019

Uh oh!

Uh oh!

m.request not redrawing automatically in Chrome #2426

m.request not redrawing automatically in Chrome #2426

Comments

sagebind commented Jun 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mithril Version: 1.1.6

Expected Behavior

Current Behavior

Possible Solution

Steps to Reproduce

Context

Your Environment

spacejack commented Jun 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sagebind commented Jun 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spacejack commented Jun 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sagebind commented Jun 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spacejack commented Jun 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

osban commented Jun 8, 2019

Uh oh!

dead-claudia commented Jun 9, 2019

Uh oh!

spacejack commented Jun 9, 2019

Uh oh!

dead-claudia commented Jun 10, 2019

Uh oh!

dead-claudia commented Jun 10, 2019

Uh oh!

spacejack commented Jun 10, 2019

Uh oh!

sagebind commented Jun 10, 2019

Uh oh!

dead-claudia commented Jun 10, 2019

Uh oh!

sagebind commented Jun 7, 2019 •

edited

Loading

spacejack commented Jun 8, 2019 •

edited

Loading

sagebind commented Jun 8, 2019 •

edited

Loading

spacejack commented Jun 8, 2019 •

edited

Loading

sagebind commented Jun 8, 2019 •

edited

Loading

spacejack commented Jun 8, 2019 •

edited

Loading