Top-level await integration #4352

littledan · 2019-02-10T01:52:14Z

The purpose of this patch is to nail down how HTML would
adjust to the needs of top-level await. The relevant change is,
the module.Evaluate() method will return a Promise that resolves
when the module is ready, and this will be passed to
FinishDynamicImport to wait on before resolving the Promise it returns.
It doesn't seem like anyone else needs to wait for a module script
to "finish" evaluating besides dynamic import.

At least two implementers are interested (and none opposed):
- Chrome
- Firefox
- Safari
Tests are written and can be reviewed and commented upon at:
- [blink] Add top-level-await tests web-platform-tests/wpt#25532
Implementation bugs are filed:
- Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=1022182
- Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1519100
- Safari: https://bugs.webkit.org/show_bug.cgi?id=202484

(See WHATWG Working Mode: Changes for more details.)

/infrastructure.html ( diff )
/references.html ( diff )
/webappapis.html ( diff )

littledan · 2019-02-10T01:52:32Z

Twin dynamic import PR tc39/proposal-dynamic-import#71

For concreteness, this patch specifies how the WebAssembly JavaScript module integration proposal [1] could work in HTML. It is not yet ready to merge, as the proposal is still in a relatively early state. Note that this change depends on the ability for modules to block in the evaluation phase, to permit WebAssembly module instantiation to yield, as is necessary on some platforms where compilation work is performed during the first instantiation. Such an ability to yield is provided by the JavaScript top-level await proposal [2] and associated HTML integration patch whatwg#4352. [1] https://github.com/webassembly/esm-integration [2] https://github.com/tc39/proposal-top-level-await

littledan · 2019-03-20T12:45:18Z

source

@@ -87776,35 +87780,26 @@ interface <dfn>ApplicationCache</dfn> : <span>EventTarget</span> {
     data-x="concept-script-record">record</span>.</p>

     <li>
-      <p>Set <var>evaluationStatus</var> to <var>record</var>.<span
+      <p>Let <var>evaluationPromise</var> be <var>record</var>.<span


Note, nobody waits on this Promise (except possibly JS code called by FinishDynamicImport).

littledan · 2019-04-16T22:44:56Z

From discussion with @domenic and @nyaxt , a few changes to this PR are planned:

The document.write counter will be decremented when the Promise of the module settles (EDIT: this refers to the ignore-destructive-writes counter)
currentScript will be restored synchronously after evaluating the module
Editorial: The extent of spec text that deals with something which is maybe a Promise, maybe a completion, will be limited to just one spot to make it a Promise (hopefully in JS, but failing that, centralized within HTML)

Ms2ger · 2019-05-07T14:35:39Z

The document.write counter will be decremented when the Promise of the module settles

Even if the module is completely sync? Would that be an observable change?

currentScript will be restored synchronously after evaluating the module

After starting to evaluate the module, but not waiting until the evaluation is finished, right?

littledan · 2019-05-07T14:50:14Z

Even if the module is completely sync? Would that be an observable change?

Oh, hmm, I guess it would be for the case where the JS stack is non-empty (which blocks the microtask checkpoint). I am not sure whether this would be a web compatibility problem, though.

Ms2ger · 2019-05-16T15:46:22Z

The document.write counter will be decremented when the Promise of the module settles

I'm not even sure this is desirable for the async case. Consider

<script type=module>
await new Promise(() => {})
</script>
<script>
setInterval(() => document.write("FAIL"), 1000);
</script>
xxx

In this case, the document.write() call will never succeed. That seems like a very surprising effect.

domenic · 2019-05-16T16:23:58Z

"Desirable" is relative. Someone did something weird, and now a weird legacy discouraged feature of the web platform (document.write) fails, in a loud way. I'm OK with that, personally.

littledan · 2019-05-16T16:27:49Z

@domenic Where's the right line? I guess none of us are proposing that the document.write counter stay incremented forever once a module is processed. Why decrement it after the TLA, and neither earlier nor later?

domenic · 2019-05-16T17:48:59Z

In cases like these, I'm happy for the line to be governed by what's easier to spec/implement.

littledan · 2019-05-19T21:49:48Z

I'm trying to follow up on #4352 (comment) . I've landed a revert 4ed3b12 in conjunction with tc39/proposal-top-level-await#74 , which makes Evaluate() always return a Promise (thanks @guybedford!), so this should reduce a bit of the incidental complexity in the previous revision.

About the other items to update, I'm having trouble piecing together what we had previously identified as things to update:

For the throw-on-dynamic-markup-insertion counter: This seems to be related to custom elements, not to executing modules. I'm not sure how we should use it here in a web-compatible way.
For currentScript, from the "execute the script block" algorithm, it looks like this is never updated for modules. So I'm not sure what we should do with top-level await.

domenic · 2019-05-20T19:24:37Z

For the throw-on-dynamic-markup-insertion counter: This seems to be related to custom elements, not to executing modules. I'm not sure how we should use it here in a web-compatible way.

Sorry, the relevant counter here is the "ignore-destructive-writes counter"

For currentScript, from the "execute the script block" algorithm, it looks like this is never updated for modules. So I'm not sure what we should do with top-level await.

This was a recent refactoring with d329a0f. That refactoring implies the clear path is to just not do anything with currentScript for top-level await, indeed.

littledan · 2019-05-21T05:40:04Z

Ah, thanks for clarifying. I updated this PR to decrement the ignore-destructive-writes counter just when it settles.

For concreteness, this patch specifies how the WebAssembly JavaScript module integration proposal [1] could work in HTML. It is not yet ready to merge, as the proposal is still in a relatively early state. Note that this change depends on the ability for modules to block in the evaluation phase, to permit WebAssembly module instantiation to yield, as is necessary on some platforms where compilation work is performed during the first instantiation. Such an ability to yield is provided by the JavaScript top-level await proposal [2] and associated HTML integration patch whatwg#4352. [1] https://github.com/webassembly/esm-integration [2] https://github.com/tc39/proposal-top-level-await

source

Ms2ger · 2019-05-21T12:30:22Z

source

-     <var>evaluationStatus</var>.[[Value]].</p></li>
-
-     <li><p>Otherwise, <span>report the exception</span> given by
+   <li><p>If <var>rethrow errors</var> is false, then upon rejection of


"rethrow errors" no longer makes sense as an argument name.

Ms2ger · 2019-05-21T12:31:07Z

source


   <li><p><span>Clean up after running script</span> with <var>settings</var>.</p></li>

-   <li><p>Return <var>evaluationStatus</var>.</p></li>
+   <li><p>Return <var>evaluationPromise</var>.</p></li>


https://w3c.github.io/ServiceWorker/#run-service-worker-algorithm needs to be updated for this.

Tracked at w3c/ServiceWorker#1407

Ms2ger · 2019-05-21T13:45:30Z

source


     <li><p>Perform <span>FinishDynamicImport</span>(<var>referencingScriptOrModule</var>,
-     <var>specifier</var>, <var>promiseCapability</var>, <var>completion</var>).</p></li>
+     <var>specifier</var>, <var>promiseCapability</var>, <var>promise</var>).</p></li>


We should make sure tc39/proposal-dynamic-import#71 lands

Tracked at tc39/proposal-top-level-await#46

littledan · 2019-05-28T12:37:55Z

Thanks for the help and guidance, everyone. I don't have any more planned changes for this PR; is anything missing? We'll be considering top-level await for Stage 3 in the June 4, 2019 TC39 meeting, and so any more thoughts you have here would be very helpful. I'm wondering whether the relationship between top-level await and ServiceWorkers in w3c/ServiceWorker#1407 has any implications for HTML (probably not?).

annevk · 2020-11-06T16:05:14Z

It seems that html/semantics/scripting-1/the-script-element/module/evaluation-order-1.html is not up-to-date with the discussion above as it expects the microtask from the module script to happen after the global onerror (also triggered by the module script). It would be good if @camillobruni could go through all the tests again.

camillobruni · 2020-11-06T17:24:59Z

It's on my TODO list for while to double check the tests again. Thanks for the pointer.

domenic · 2020-11-06T17:48:55Z

We also need to integrate top-level await and worklets, since worklets ended up landing in the HTML spec before this PR did. From what I understand you've been doing that implementation work, @camillobruni; would you be able to outline a rough sketch of how it's implemented? I can try to finish up the spec for it.

In particular I'm unsure what the final decision was on the question in w3c/css-houdini-drafts#984 (comment) , as to how we run multiple top-level-await-using scripts into a new worklet global scope.

codehag · 2020-11-18T17:35:35Z

I have done a bit of testing with this. I am a bit stuck about what to do because I am not very familiar with the html spec. Sorry this is super beginner stuff, I just don't know where to look in the spec to answer my questions.

My naive implementation suggests that html/semantics/scripting-1/the-script-element/module/evaluation-order-1.html needs to be adjusted, and that the microtask from the module script should happen before global onerror (as pointed out by annevk).

But looking at how chrome handles it -- this test appears to pass? I ran it with chrome canary with the --top-level-await flag here: https://bugs.chromium.org/p/v8/issues/detail?id=9344, and also with experimental features turned on which seems to have the same effect. Top level await appears to be turned on in that case.

Oddly, the html/semantics/scripting-1/the-script-element/module/evaluation-order-4-tla.html test fails to run, and times out in chrome, and on my implementation. In this case, my naive implementation will pass this test if the following is changed:

              "step-4.1-1", "step-4.1-2", "microtask-4.1",
             "script-load", "window-load",
             "step-4.2-1", "step-4.2-2", "microtask-4.2",

to this:

              "step-4.1-1", "step-4.1-2", "microtask-4.1",
             "script-load",
             "step-4.2-1", "step-4.2-2", "microtask-4.2",
             "window-load",

While it fixes my problem, this times out with chrome as well. So, where I am stuck is I am not too sure where to look to figure out which behaviour is right. If it turns out that html/semantics/scripting-1/the-script-element/module/evaluation-order-1.html is correct, and that only html/semantics/scripting-1/the-script-element/module/evaluation-order-4-tla.html is wrong, I will adjust my implementation. Any guidance would be appreciated.

annevk

While looking at this PR again I found some minor nits.

source

annevk · 2020-11-19T12:23:36Z

@codehag I don't see anything in the specification that would further delay the main load event so that looks wrong. Perhaps Firefox counts (dynamic) import() as further delaying it, but import() does not block the main load event per the specification. (That could be a bug in the specification as XMLHttpRequest would further delay it, iirc. I guess that would warrant a bit more testing as to what browsers do for import() in general.)

annevk · 2020-11-20T09:46:28Z

@domenic looking at this again I don't think changes to worklets are needed (they'll continue to invoke run the module script and ignore the result and as long as their global doesn't implement EventTarget won't hear about exceptions), but WebAudio/web-audio-api#2267 might need some work.

domenic · 2020-11-20T16:23:30Z

@domenic looking at this again I don't think changes to worklets are needed (they'll continue to invoke run the module script and ignore the result and as long as their global doesn't implement EventTarget won't hear about exceptions), but WebAudio/web-audio-api#2267 might need some work.

This is one possible behavior. The other possible behavior is to have each module script wait for the previous one's to complete. That latter behavior would need spec changes, I believe. And we haven't gotten any answer on which behavior we want.

I believe @camillobruni is writing some worklet tests, but I don't know which behavior he's testing.

annevk · 2020-11-20T16:27:57Z

I think your take that it shouldn't be different from multiple <script type=module>s is correct, but maybe there is a very compelling story to do something different?

syg · 2020-11-20T19:50:03Z

Re: html/semantics/scripting-1/the-script-element/module/evaluation-order-1.html

The way this test is currently done in chromium is that the timing is changed when the TLA flag is passed to the renderer. There's a virtual test suite that changes the timing expectations to the one described by this PR if the TLA flag is on:
https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/web_tests/virtual/module-top-level-await/external/wpt/html/semantics/scripting-1/the-script-element/module/evaluation-order-1-expected.txt;l=1?q=evaluation-order-1-expected.txt&sq=&ss=chromium

The base WPT doesn't have the TLA flag, so it has the old timing. I imagine the test will be updated once the TLA flag is flipped on by default.

To clarify @annevk's question: are you asking for the test to be updated ahead of time?

To clarify @codehag's comment: the chromium implementation follows this PR, but only under the flag.

hiroshige-g · 2020-11-22T03:52:04Z

@codehag I don't see anything in the specification that would further delay the main load event so that looks wrong. Perhaps Firefox counts (dynamic) import() as further delaying it, but import() does not block the main load event per the specification. (That could be a bug in the specification as XMLHttpRequest would further delay it, iirc. I guess that would warrant a bit more testing as to what browsers do for import() in general.)

This is discussed in #5824.

annevk · 2020-11-30T13:34:57Z

@syg typically there's at least a PR open for test changes that can be merged together with the change to the standard. See https://whatwg.org/working-mode#changes.

syg · 2020-12-02T00:12:53Z

@syg typically there's at least a PR open for test changes that can be merged together with the change to the standard. See https://whatwg.org/working-mode#changes.

I was imagining this would be done via the WPT 2-way sync if Chromium flips it on first. Is that acceptable?

syg · 2020-12-02T00:49:01Z

How is this envisioned to work with synthetic module records, btw? I imagine all synthetic module records will also change to always returning promises?

annevk · 2020-12-02T06:17:28Z

@syg that's fine, assuming there's some diff for editors to review.

domenic · 2020-12-02T15:58:03Z

Synthetic module records were removed from Web IDL, so I don't think they're an issue any more.

domenic · 2020-12-09T20:21:29Z

web-platform-tests/wpt#26771 has been merged, which I believe contains the test updates discussed upthread. And I just wrote web-platform-tests/wpt#26826 to get us some basic worklet test coverage.

So, I think this can be merged! I'll do so tomorrow-ish, unless folks have any objections.

andreubotella · 2020-12-09T21:39:28Z

source

-      <span>"<code>QuotaExceededError</code>"</span> <code>DOMException</code>, [[Target]]: empty
-      }.</p>
+      <var>evaluationPromise</var> to <span>a promise rejected with</span> a new
+      <span>"<code>QuotaExceededError</code>"</span> <code>DOMException</code>.</p>


Can't the user agent abort a module evaluation after the first await? Unless I'm missing something, in that case evaluationPromise would never resolve.

Hmm, very interesting. So an instance of the case we're wondering about is

<script type="module"> try { await import("./bad.mjs"); } catch (e) { // e should ideally be a "QuotaExceededError" DOMException } </script>

// bad.mjs await 1; while (true) { } await 2;

Any ideas what implementations are planning to do, @codehag and @camillobruni?

Good find! I tested in Chrome that an infinite looping module can hang the renderer when dynamically imported. The slow script killer doesn't kill dynamic imports correctly, even without top-level await. That is, the following also hangs:

<script> (async function() { await import("./bad.mjs"); } catch (e) { // e should ideally be a "QuotaExceededError" DOMException } })(); </script>

// bad.mjs while (true) { }

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=1157321

I can confirm that firefox has the same behavior as what @syg described for chrome above: both the test cases result in the tab hanging.

We do show a user prompt however, about a slow script and let the user respond. When the user responds on the second use case, the script is stopped and the page becomes usable again. Crashes with TLA though.

The bug tracking this on the firefox side is https://bugzilla.mozilla.org/show_bug.cgi?id=1681664

cc @annevk

Alright. A simpler example without dynamic import is the following:

<script> window.onerror = event => { console.log(event.error); // should ideally log a "QuotaExceededError" DOMException }; </script> <script type="module"> await 1; while (true) { } </script>

I'll try and work on some spec text giving UAs license to terminate top-level-await using scripts in the same way they are currently given license to terminate other scripts, and I'll be sure it also covers dynamic import(). I might merge this first if the fix seems especially separable.

This is looking like more of a mess than I thought. The script-killing section of the spec isn't very well defined or aligned with UAs; e.g. as far as I can tell from Chrome it kills the entire process after prompting, which is not really allowed by the current spec text. And we're not sure if any browser actually bothers with the "QuotaExceededError" DOMException business.

So I'll merge this as-is and file a followup bug.

For concreteness, this patch specifies how the WebAssembly JavaScript module integration proposal [1] could work in HTML. It is not yet ready to merge, as the proposal is still in a relatively early state. Note that this change depends on the ability for modules to block in the evaluation phase, to permit WebAssembly module instantiation to yield, as is necessary on some platforms where compilation work is performed during the first instantiation. Such an ability to yield is provided by the JavaScript top-level await proposal [2] and associated HTML integration patch whatwg#4352. [1] https://github.com/webassembly/esm-integration [2] https://github.com/tc39/proposal-top-level-await

- Enable tests with TLA enabled - Fix top-level error reporting with TLA Based on the discussions on the spec, we fire error events on rejection of the result promise: whatwg/html#4352 (comment) Bug: 1022182, 1096455, 1127215, v8:9344 Change-Id: I16e83cb4e279c1e44be7fa70a51a103ee94aacc3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2228881 Commit-Queue: Camillo Bruni <cbruni@chromium.org> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Reviewed-by: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/master@{#806593} GitOrigin-RevId: 077d63da3268e6761d20c56409d37c57d37f1685

This reverts commit 077d63da3268e6761d20c56409d37c57d37f1685. Reason for revert: https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Linux%20Leak is constantly failing on * virtual/module-top-level-await/external/wpt/html/semantics/scripting-1/the-script-element/module/evaluation-order-4-tla.html * virtual/module-top-level-await/external/wpt/html/webappapis/dynamic-markup-insertion/document-write/module-tla-delayed.html * virtual/module-top-level-await/external/wpt/html/webappapis/dynamic-markup-insertion/document-write/module-tla-immediate-promise.html * virtual/module-top-level-await/external/wpt/html/webappapis/dynamic-markup-insertion/document-write/module-tla-import.html Original change's description: > [blink] Fix top-level-await error reporting > > - Enable tests with TLA enabled > - Fix top-level error reporting with TLA > > Based on the discussions on the spec, we fire error events on rejection > of the result promise: > whatwg/html#4352 (comment) > > Bug: 1022182, 1096455, 1127215, v8:9344 > Change-Id: I16e83cb4e279c1e44be7fa70a51a103ee94aacc3 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2228881 > Commit-Queue: Camillo Bruni <cbruni@chromium.org> > Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> > Reviewed-by: Kouhei Ueno <kouhei@chromium.org> > Reviewed-by: Dominic Farolino <dom@chromium.org> > Reviewed-by: Domenic Denicola <domenic@chromium.org> > Cr-Commit-Position: refs/heads/master@{#806593} TBR=hiroshige@chromium.org,kouhei@chromium.org,cbruni@chromium.org,dom@chromium.org,domenic@chromium.org Change-Id: I66b085b05ad30020772e80324e226275e41a8782 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 1022182 Bug: 1096455 Bug: 1127215 Bug: v8:9344 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410725 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#806724} GitOrigin-RevId: 593ef19ebb9255397222180a3bacf9a78bfbbffd

- Add better assertion failure messages - Use better content for document.write The tests assume that the ignore-destructive-writes counter is only incremented during the synchronous part of module evaluation (see whatwg/html#4352 (comment)). This also fixes document-write/module-delayed.html which accidentally had wrong test expectations checked in that made it pass. Bug: 1127215 Bug: 1022182 Bug: v8:9344 Change-Id: I01a75534f7efd0bd8e376dfd049432e52661604d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2397696 Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Commit-Queue: Camillo Bruni <cbruni@chromium.org> Cr-Commit-Position: refs/heads/master@{#807384} GitOrigin-RevId: 8168e512fcecd52c45ffe99b425b9e392959f895

This is a reland of 077d63da3268e6761d20c56409d37c57d37f1685 - Add missing test expections Original change's description: > [blink] Fix top-level-await error reporting > > - Enable tests with TLA enabled > - Fix top-level error reporting with TLA > > Based on the discussions on the spec, we fire error events on rejection > of the result promise: > whatwg/html#4352 (comment) > > Bug: 1022182, 1096455, 1127215, v8:9344 > Change-Id: I16e83cb4e279c1e44be7fa70a51a103ee94aacc3 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2228881 > Commit-Queue: Camillo Bruni <cbruni@chromium.org> > Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> > Reviewed-by: Kouhei Ueno <kouhei@chromium.org> > Reviewed-by: Dominic Farolino <dom@chromium.org> > Reviewed-by: Domenic Denicola <domenic@chromium.org> > Cr-Commit-Position: refs/heads/master@{#806593} TBR=kouhei@chromium.org Bug: 1022182 Bug: 1096455 Bug: 1128296 Bug: 1127215 Bug: v8:9344 Change-Id: Icecaccf34efc4354e0faac362da6f6cbdd49f50d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410075 Reviewed-by: Dominic Farolino <dom@chromium.org> Commit-Queue: Dominic Farolino <dom@chromium.org> Cr-Commit-Position: refs/heads/master@{#808128} GitOrigin-RevId: 5e17eba0f8ba762b3e9ce72052057f56ba0f716d

littledan mentioned this pull request Feb 10, 2019

[WIP] Top-level await integration tc39/proposal-dynamic-import#71

Closed

This was referenced Feb 10, 2019

Various editorial improvements guybedford/proposal-top-level-await#1

Merged

Tracking: HTML integration tc39/proposal-top-level-await#45

Closed

This was referenced Feb 16, 2019

[WIP] WebAssembly JavaScript Module integration #4372

Closed

Avoid reordering execution of module bodies before top-level await is reached tc39/proposal-top-level-await#47

Closed

annevk added addition/proposal New features or enhancements do not merge yet Pull request must not be merged per rationale in comment integration Better coordination across standards needed labels Mar 1, 2019

littledan mentioned this pull request Mar 20, 2019

Top-level await integration and multiple script graphs #4419

Closed

littledan commented Mar 20, 2019

View reviewed changes

Ms2ger force-pushed the top-level-await branch from 8415c83 to 2204867 Compare May 17, 2019 11:01

Ms2ger reviewed May 21, 2019

View reviewed changes

littledan changed the title ~~[WIP] Top-level await integration~~ Top-level await integration May 28, 2019

annevk reviewed Nov 19, 2020

View reviewed changes

source Show resolved Hide resolved

source Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Show resolved Hide resolved

Fix review comments.

714a7e7

domenic mentioned this pull request Dec 9, 2020

Test top-level await in worklets web-platform-tests/wpt#26826

Merged

andreubotella reviewed Dec 9, 2020

View reviewed changes

domenic merged commit fee73fa into whatwg:master Dec 10, 2020

domenic mentioned this pull request Dec 10, 2020

It's unclear how well browsers line up with the "Killing scripts" section of the spec #6210

Open

hiroshige-g mentioned this pull request Jan 2, 2023

Yielding to the event loop (in some way) between module evaluations #4400

Closed

Top-level await integration #4352

Top-level await integration #4352

Conversation

littledan commented Feb 10, 2019 • edited by pr-preview bot Loading

littledan commented Feb 10, 2019

Choose a reason for hiding this comment

littledan commented Apr 16, 2019 • edited Loading

Ms2ger commented May 7, 2019

littledan commented May 7, 2019

Ms2ger commented May 16, 2019

domenic commented May 16, 2019

littledan commented May 16, 2019

domenic commented May 16, 2019

littledan commented May 19, 2019

domenic commented May 20, 2019

littledan commented May 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledan commented May 28, 2019

annevk commented Nov 6, 2020

camillobruni commented Nov 6, 2020

domenic commented Nov 6, 2020

codehag commented Nov 18, 2020

annevk left a comment

Choose a reason for hiding this comment

annevk commented Nov 19, 2020

annevk commented Nov 20, 2020

domenic commented Nov 20, 2020

annevk commented Nov 20, 2020

syg commented Nov 20, 2020

hiroshige-g commented Nov 22, 2020

annevk commented Nov 30, 2020

syg commented Dec 2, 2020

syg commented Dec 2, 2020

annevk commented Dec 2, 2020

domenic commented Dec 2, 2020

domenic commented Dec 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syg Dec 10, 2020 • edited Loading

Choose a reason for hiding this comment

codehag Dec 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledan commented Feb 10, 2019 •

edited by pr-preview bot

Loading

littledan commented Apr 16, 2019 •

edited

Loading

syg Dec 10, 2020 •

edited

Loading

codehag Dec 10, 2020 •

edited

Loading