Closed rekmarks closed 1 week ago
Attention: Patch coverage is 78.57143%
with 6 lines
in your changes are missing coverage. Please review.
Project coverage is 67.46%. Comparing base (
d27a233
) to head (38cf3c7
). Report is 50 commits behind head on develop.:exclamation: Current head 38cf3c7 differs from pull request most recent head 2c1d1a3. Consider uploading reports for the commit 2c1d1a3 to get more accurate results
Files | Patch % | Lines |
---|---|---|
...ib/rpc-method-middleware/createMethodMiddleware.js | 77.78% | 6 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Platform | Page | Metric | Min (ms) | Max (ms) | Average (ms) | StandardDeviation (ms) | MarginOfError (ms) |
---|---|---|---|---|---|---|---|
Chrome | Home | firstPaint | 55 | 124 | 73 | 14 | 7 |
domContentLoaded | 7 | 16 | 10 | 2 | 1 | ||
load | 44 | 2691 | 659 | 1048 | 503 | ||
domInteractive | 7 | 16 | 10 | 2 | 1 |
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.
Platform | Page | Metric | Min (ms) | Max (ms) | Average (ms) | StandardDeviation (ms) | MarginOfError (ms) |
---|---|---|---|---|---|---|---|
Chrome | Home | firstPaint | 54 | 109 | 70 | 12 | 6 |
domContentLoaded | 8 | 61 | 12 | 11 | 5 | ||
load | 43 | 2475 | 297 | 719 | 345 | ||
domInteractive | 8 | 60 | 12 | 11 | 5 |
Platform | Page | Metric | Min (ms) | Max (ms) | Average (ms) | StandardDeviation (ms) | MarginOfError (ms) |
---|---|---|---|---|---|---|---|
Chrome | Home | firstPaint | 58 | 143 | 83 | 21 | 10 |
domContentLoaded | 8 | 26 | 12 | 4 | 2 | ||
load | 47 | 3518 | 634 | 1152 | 553 | ||
domInteractive | 8 | 26 | 12 | 4 | 2 |
Platform | Page | Metric | Min (ms) | Max (ms) | Average (ms) | StandardDeviation (ms) | MarginOfError (ms) |
---|---|---|---|---|---|---|---|
Chrome | Home | firstPaint | 75 | 143 | 102 | 22 | 11 |
domContentLoaded | 10 | 63 | 18 | 12 | 6 | ||
load | 61 | 3725 | 984 | 1392 | 668 | ||
domInteractive | 10 | 63 | 18 | 12 | 6 |
It does seem like this could be handled more consistently at the type level by TypeScript, but I'm assuming the issue is that there is still a lot of unmigrated js code that will benefit from this in the meantime?
@legobeat there's definitely unmigrated JS code, but also the types for the method middleware and its handler + hooks pattern are currently broken and implemented in 3 different repositories. 🙃
We're all trying to find the guy who did this...
I may try to wrangle the types once this is in.
Platform | Page | Metric | Min (ms) | Max (ms) | Average (ms) | StandardDeviation (ms) | MarginOfError (ms) |
---|---|---|---|---|---|---|---|
Chrome | Home | firstPaint | 68 | 142 | 88 | 22 | 11 |
domContentLoaded | 9 | 39 | 15 | 9 | 4 | ||
load | 55 | 3067 | 736 | 1172 | 563 | ||
domInteractive | 9 | 39 | 15 | 9 | 4 |
Platform | Page | Metric | Min (ms) | Max (ms) | Average (ms) | StandardDeviation (ms) | MarginOfError (ms) |
---|---|---|---|---|---|---|---|
Chrome | Home | firstPaint | 59 | 143 | 84 | 20 | 9 |
domContentLoaded | 9 | 28 | 13 | 6 | 3 | ||
load | 47 | 2893 | 745 | 1181 | 567 | ||
domInteractive | 9 | 28 | 13 | 6 | 3 |
Description
Removes RPC handler hooks that had become unused. In order to prevent this from occurring again, also adds a check to fail immediately on dapp connections if extraneous hooks are provided. Finally, adds tests for
createMethodMiddleware.js
.Note for reviewers
Pre-merge author checklist
Pre-merge reviewer checklist