Use import() on esm files in supported node versions #547

lkmill · 2021-02-22T09:05:13Z

This is a proof-of-concept of using import() to load ESM files in supported node versions (>= 14, >= 13.2.0, >= 12.17.0). The syntax breaking import() call is located in a separate file from bin/tape.js that is conditionally loaded, preventing older Node versions from crashing.

This is currently an unfinished draft. I'm mainly curious to see if you are interested in something like this.

Currently this branch does not use promises to sequentially load the imported files. I have a separate branch, https://github.com/lohfu/tape/blob/esm-support-promises/bin/load-module-esm-support.js#L27-L29, where I have implemented sequential loading but this causes a test repo with a couple of simple test files to fail.

Questions / Caveats

What is needed for the sequential loading to work? It seems like tape finishes before all tests are run properly. In a very simple repo with 2 test files all tests in the second file error with not ok 2 test exited without ending: file 2 test 1. The tests simply contain a single t.pass and a t.end. All tests in the tape repo pass.
Currently only ${cwd}/package.json is loaded as I was a little reluctant to add a new dependency to find closest package.json... Is this ok? Do you have any preferences?
Is there a better way to determine ESM support than checking the node version?
nyc instrumentation fails: failed to instrument /home/lohfu/dev/tape/bin/load-module-esm-support.js with error: SyntaxError: 'import' and 'export' may only appear at the top level (14:8)... Can I update tap to get a newer nyc?
Currently only .mjs and .js files are loaded using import(). This prevents the use of loaders on other file extensions. In the future an --extension argument will probably be needed in the cli to allow for loaders on custom extensions.
Is the bin file ever bundled into browsers? I presume bundlers will add the file with the import to the bundle, which will cause older browsers to SyntaxError.

codecov-io · 2021-02-22T09:54:58Z

Codecov Report

Merging #547 (bfaec0a) into master (50751db) will decrease coverage by 1.90%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #547      +/-   ##
==========================================
- Coverage   74.65%   72.75%   -1.91%     
==========================================
  Files          19       20       +1     
  Lines         789      800      +11     
  Branches      151      153       +2     
==========================================
- Hits          589      582       -7     
- Misses        200      218      +18

Impacted Files	Coverage Δ
bin/load-module-esm-support.js	`0.00% <0.00%> (ø)`
lib/test.js	`94.30% <0.00%> (-1.43%)`	⬇️
index.js	`96.73% <0.00%> (-1.09%)`	⬇️
lib/results.js	`99.31% <0.00%> (-0.69%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50751db...bfaec0a. Read the comment docs.

ljharb

I agree that it's important these be serial - so when import() is supported, i think the entire thing should be a .reduce using Promises.

bin/load-module-esm-support.js

bin/tape

ljharb · 2021-02-23T15:29:02Z

We also can’t upgrade tap due to the node versions it drops.

lkmill · 2021-02-23T15:57:24Z

i used a promise reducer to load test files serially and added failing tests. the new tests fail due to the problem i explained in the PR description, namely all tests in the files loaded after the first fail with not ok 2 test exited without ending: module-b

We also can’t upgrade tap due to the node versions it drops.

hmmm, ok. any other way of circumventing the instrumentation error?

ljharb · 2021-02-23T18:44:49Z

I'm not sure what the instrumentation error is; is it about import()?

lkmill · 2021-02-23T21:04:51Z

regarding the failing tests, if you point me in the general direction of possible causes i am keen to start debugging and looking for solutions.

I'm not sure what the instrumentation error is; is it about import()?

yes, the exact error is:

failed to instrument /home/lohfu/dev/tape/bin/load-module-esm-support.js with error: SyntaxError: 'import' and 'export' may only appear at the top level (16:8)

ljharb · 2021-02-23T21:49:55Z

add bin/load-module-esm-support.js to exclude in .nycrc?

ljharb · 2021-02-23T21:50:36Z

also we'll need to make sure the tests that do imports are skipped in the appropriate node versions.

bin/load-module-esm-support.js

bin/tape

haroun

@lohfu what do you think about the following changes? I guess it will not work in node versions without ESM support, but still I’ve tried it on node 14.15.3 and seems to make the tests go green. If you can help me understand what I’m missing, maybe I can do the changes. Thank you so much.

test/import.js

bin/tape

lib/results.js

lkmill · 2021-05-11T12:54:18Z

@ljharb All tests pass now. If you are ok with the way i wait for all tests to load before running them I reckon the only things left now are:

Skipping import tests in appropriate Node versions
~~Deciding whether to check to use require or import for each file or always using import.~~

lkmill · 2021-05-11T13:39:44Z

Ok I did some very basic performance testing to see the implications of always using import. I created 10,000 empty .js files with a module type package.json. There was no observable performance benefit of removing the package type check and always using import. However, require:ing the same 10,000 files was around 4 times faster than importing them. Conclusion: no benefits when importing ESM files but serious disadvantage when importing CommonJS files.

lkmill · 2021-05-13T14:29:42Z

@ljharb what do you think about using this solution instead of checking which version of node is used to figure out if dynamic import is supported?

https://github.com/davidtheclark/cosmiconfig/pull/251/files#diff-e61e569af4ed5c2bd19c648e11225dd502296f0931b339c88cb82288a493e6ad

ljharb · 2021-05-13T17:51:07Z

@lohfu that's basically the solution i'll be using in the package i forgot to create :-) let me get on that today.

codecov-commenter · 2021-05-13T17:52:42Z

Codecov Report

Merging #547 (e6d2faf) into master (9bbf270) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #547      +/-   ##
==========================================
+ Coverage   96.70%   96.77%   +0.06%     
==========================================
  Files           4        4              
  Lines         607      620      +13     
  Branches      144      147       +3     
==========================================
+ Hits          587      600      +13     
  Misses         20       20

Impacted Files	Coverage Δ
index.js	`98.11% <100.00%> (+0.26%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9bbf270...e6d2faf. Read the comment docs.

ljharb · 2021-05-14T04:01:24Z

@lohfu alrighty, give https://npmjs.com/has-dynamic-import a spin

lkmill · 2021-05-14T13:28:59Z

@lohfu alrighty, give https://npmjs.com/has-dynamic-import a spin

Sweet! Unfortunately I ran into some trouble with it immediately... I use has-dynamic-import to skip the ESM tests, and currently they arent skipped and fail in Node 10, 11, < 12.17 and < 13.2. All else seem to work fine. I created an issue and a PR explaining the issue and possible workaround.

lkmill · 2021-05-30T20:43:51Z

i have fiddled around a little with getting this PR working with inspect-js/has-dynamic-import#3. all tests seem to be working on my end, but i have the following questions:

is the current way of handling async module loading ok? see:

should we have an option for custom extensions that enforces import() like .mjs does now? this would enable using loaders and other spooky magic (https://github.com/substack/tape/pull/547/files#diff-79dfc9fc6fa8fb5312506d39301093b519b95edbb72465a85102b1be7ec0f0f7R9)
should i squash/rebase away all "useless" commits or do you want the full history?

lkmill · 2021-05-30T20:55:37Z

since full support check is now async and the tap skip option is sync, i used async sub-tests to exclude import() tests in unsupported environments (https://github.com/substack/tape/pull/547/files#diff-5ab063121df04838565bf235950a645d72c689af2409b48129368b6317bf23b2R8-R52), is this a viable solution?

ljharb

Re 4, I think the t.plan(2) has to happen synchronously, otherwise tap has no idea that those tests are incoming.

Re 1, I'm not a fan of having Results.__loadingPromise exposed (see https://github.com/substack/tape/pull/547/files#r630197895)

Re 2, I'm not sure what you mean. "loaders" are already possible for CJS via require.extensions.

Re 3, please feel free to squash/rebase as needed.

package.json

lkmill

Re 4, I think the t.plan(2) has to happen synchronously, otherwise tap has no idea that those tests are incoming.

seemed to work fine, but i converted to using t.end() just in case.

Re 1, I'm not a fan of having Results.__loadingPromise exposed (see https://github.com/substack/tape/pull/547/files#r630197895)

can't really think of any other solution... got any ideas?

Re 2, I'm not sure what you mean. "loaders" are already possible for CJS via require.extensions.

I was referring to ESM loaders... in the not too distant future i suspect some people want to import() files beyond .mjs or .js.

ljharb · 2021-05-31T20:47:15Z

When node supports something more, we can add support for it. At the moment, there are no other kinds of modules.

bin/import-or-require.js

bin/tape

lib/results.js

ljharb

It occurred to me today that node only has --require and does not have any way to evaluate ESM files on process load - it might be better to wait until node has that support before adding it here, in case they conflict.

eg, it's possible node will add --import= for ESM, in which case that's what we'd want to have.

lkmill · 2021-06-01T18:48:20Z

It occurred to me today that node only has --require and does not have any way to evaluate ESM files on process load - it might be better to wait until node has that support before adding it here, in case they conflict.

removed!

ljharb · 2021-06-02T05:10:12Z

I'm not sure what you removed - what i mean is, --require=./foo.mjs is impossible in node, so i'm wondering if it should remain impossible in tape.

lkmill · 2021-06-02T12:19:28Z

i removed the logic to use import() statements on ESM file in the --require option.

i am feeling pretty ready with this PR now... if you are ok with the wait()/run() logic for handling async importing of tests before running them i will update the README and squash everything to a single commit.

ljharb

Definitely getting close!

bin/tape

index.js

lib/results.js

ljharb · 2021-06-02T15:21:42Z

package.json

@@ -27,8 +27,10 @@
        "defined": "^1.0.0",
        "dotignore": "^0.1.2",
        "for-each": "^0.3.3",
+        "get-package-type": "^0.1.0",


I’m a bit concerned that this package’s engines declaration will cause false errors for yarn users, and anyone using engines-strict or https://npmjs.com/ls-engines, if it ever adds an engines declaration in the future (cc @coreyfarrell)

bin/tape

index.js

bin/tape

ljharb · 2021-06-02T20:49:35Z

bin/tape

+          }
+        );
+    } else {
+        tape.run();


in a node without Promise, will this error just be surfaced to the process?

yes, that works. however, it apparently fails if Node supports a promise, but no import() call has been made. since hasDynamicImport creates a promise, we need to move the catch outside. I had to rethink the logic a bit... i'm beginning to feel ashamed 😞 ... but changed and pushed.

i should probably add some tests for this error handling.

bin/import-or-require.js

index.js

.eslintrc

bin/tape

ljharb

i've rebased the PR; please fetch before making further changes.

bin/tape

.eslintrc

lkmill · 2021-06-06T22:39:51Z

so i started using the changes in this branch on a project, and i've found a major drawback to the handling of rejections when loading test files.

https://github.com/substack/tape/blob/229bd7876ec71d5c8d7681a4f2e6c13cb61a0efe/bin/tape#L67-L73

catching errors in our loading promise, console.error:ing them and then exiting makes it very difficult to locate syntax errors in ESM files. running an ESM file with invalid syntax "natively" in Node prints:

$ node tmp.mjs
file:///home/notme/dev/tape/tmp.mjs:1
)
^

SyntaxError: Unexpected token ')'
    at Loader.moduleStrategy (node:internal/modules/esm/translators:147:18)
    at async link (node:internal/modules/esm/module_job:64:21)

the actual stack trace contains no information of where the syntax error is; it's the lines before that discloses the error and its location. running the same file with this branch prints:

$ ./bin/tape tmp.mjs
SyntaxError: Unexpected token ')'
    at Loader.moduleStrategy (node:internal/modules/esm/translators:147:18)
    at async link (node:internal/modules/esm/module_job:64:21)

with only that information, it obviously becomes difficult to locate the problem without external tools. node appears to behave differently when the syntax errors are in commonjs files, ie when tape calls require() and not import(). in that case, console.error(err) prints the syntax error and it's location:

$ ./bin/tape tmp.cjs
/home/notme/dev/tape/tmp.cjs:1
)
^

SyntaxError: Unexpected token ')'
    at Object.compileFunction (node:vm:353:18)
    at wrapSafe (node:internal/modules/cjs/loader:1039:15)
    at Module._compile (node:internal/modules/cjs/loader:1073:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1138:10)
    at Module.load (node:internal/modules/cjs/loader:989:32)
    at Function.Module._load (node:internal/modules/cjs/loader:829:14)
    at Module.require (node:internal/modules/cjs/loader:1013:19)
    at require (node:internal/modules/cjs/helpers:93:18)
    at module.exports (/home/notme/dev/tape/bin/import-or-require.js:12:5)
    at /home/notme/dev/tape/bin/tape:85:24

ljharb · 2021-06-07T01:07:46Z

Yeah, i think node does some magic with exceptions that are not caught and re-thrown. I’m not sure how to solve for that.

lkmill · 2021-06-10T23:30:45Z

After giving it some thought I have come up with some changes. The basic idea is to use process.nextTick to leave the promise chain after checking for import support and only enter into a new promise chain when loading ESM files. This way we avoid unhandled rejections when only loading CommonJS files, ie in all current setups using tape. If we do enter into a promise chain when loading files, rejections are no longer caught. This makes Node 15 onward handle rejections importing files similar to errors requiring files. I know one should usually strive for handling all rejections, but I reckon this is a bit of an edge case. We want the program to quit when there are rejections importing files, and we want to show exactly what Node natively prints. If rejections need to be handled when importing files, shouldn't the current require calls be wrapped in a try/catch?

After these last changes, this PR only affects people trying to load ESM files in versions that supports it and a loading promise will only be created if an ESM file is imported. This completely avoids unhandled rejections when using CommonJS and will only have awkward output in Node versions before 15. People converting their projects to full ESM most likely use very modern Node and as such, this should have poor behaviour for few. Moreover, 99% of unhandled rejections importing files will be syntax errors and developers have tools in place to locate those.

On rejections importing ESM files, the previous changes printed::

SyntaxError: Unexpected token ')'
    at Loader.moduleStrategy (internal/modules/esm/translators.js:84:18)
    at async link (internal/modules/esm/module_job.js:36:21)

Evidently this output gives very little clue as to were the error has occurred. As just mentioned, usually these errors will easily be solved with other developer tools, but in rarer cases not getting an exact location where the error was thrown can be struggling.

After the last changes, promise rejections importing ESM files prints:

Node >=15:

file:///home/notme/dev/tape/tmp.mjs:1
)
^

SyntaxError: Unexpected token ')'
    at Loader.moduleStrategy (node:internal/modules/esm/translators:147:18)
    at async link (node:internal/modules/esm/module_job:48:21)

Node ^12.17, ^13.2 and 14:

(node:91315) UnhandledPromiseRejectionWarning: SyntaxError: Unexpected token ')'
    at Loader.moduleStrategy (internal/modules/esm/translators.js:84:18)
    at async link (internal/modules/esm/module_job.js:36:21)
(node:91315) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:91315) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

All Node versions that do not support import:

/home/notme/dev/tape/tmp.mjs:1
)
^

SyntaxError: Unexpected token )
    at Module._compile (internal/modules/cjs/loader.js:760:23)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:827:10)
    at Module.load (internal/modules/cjs/loader.js:685:32)
    at Function.Module._load (internal/modules/cjs/loader.js:620:12)
    at Module.require (internal/modules/cjs/loader.js:723:19)
    at require (internal/modules/cjs/helpers.js:14:16)
    at /home/lohfu/dev/tape/bin/tape:86:34
    at Array.forEach (<anonymous>)
    at importFiles (/home/lohfu/dev/tape/bin/tape:86:11)
    at processTicksAndRejections (internal/process/task_queues.js:81:17)

For errors requiring CJS files, the output has always been the same both inside and outside promises (both caught and uncaught).

ljharb · 2021-06-13T03:29:13Z

If rejections need to be handled when importing files, shouldn't the current require calls be wrapped in a try/catch?

It's not that they need to be handled; it's that module-level exceptions (at require/import time) should crash the process; unhandled rejections (including from an ESM module that does export function then() { throw 42 }) should not.

Ideally, a module-level exception from a CJS file should look the same in any version of node, whether or not you're including ESM files.

Your last comment implies we might be pretty close to the ideal scenario :-) I'll take a look!

lkmill · 2021-06-16T14:45:11Z

Since playing around a little with the --unhandled-rejections=mode CLI option I am getting quite convinced this is a reasonable solution. Using the NODE_OPTIONS env variable to set unhandled rejections mode to either strict or throw, Node and tape handles unhandled rejections the same in all version of Node that supports ESM imports.

$ NODE_OPTIONS='--unhandled-rejections=throw' ./bin/tape tmp.mjs
file:///home/notme/dev/tape/tmp.mjs:1
)
^

SyntaxError: Unexpected token ')'
    at Loader.moduleStrategy (node:internal/modules/esm/translators:147:18)
    at async link (node:internal/modules/esm/module_job:48:21)

I added some tests to check how the process exits, but do not really have any tests checking the error output.

EDIT:

Only Node >=14 seems to support throw mode, earlier version only support strict. in the tests i had to set it to strict mode to make node 12 pass and 13.

EDIT:

Added some tests checking for unhandled rejections in the stderr.

…with bin/tape

lkmill · 2021-07-06T10:44:49Z

@ljharb have you had any time to review the latest changes?

.eslintrc

ljharb · 2021-07-23T21:17:58Z

@lohfu thanks for your patience and for working so thoroughly on this, I really appreciate it!

I'll cut a release that includes this soon.

- [New] Use import() on esm files in supported node versions (#547) - [Refactor] `bin/tape`: separate "preparing of files list" from "require files list" - [Refactor] avoid reassigning arguments - [Refactor] remove unused line, unneeded var initialization; add missing `new` - [Refactor] remove use of legacy `exports` - [Refactor] add names to `Test.prototype` functions - [Deps] update `object-inspect` - [Docs] correct docs for `t.teardown` (#558) - [readme] Another way to create custom reporters (#556) - [readme] remove travis badge; add actions and codecov badges - [Deps] update `glob`, `is-regex`, `object-inspect` - [meta] ensure `not-in-publish`‘s absence does not fail anything - [meta] add `safe-publish-latest`; use `prepublishOnly` script for npm 7+ - [Dev Deps] update `aud`, `eslint` - [Tests] uncaught exceptions and unhandled rejections importing files with bin/tape (#547) - [Tests] exclude examples from coverage - [actions] use `node/install` instead of `node/run`; use `codecov` action

ljharb · 2021-07-26T23:47:34Z

See v5.3.0

ghost · 2021-09-20T07:40:17Z

Just here to say that this is amazing, thank you everyone 🥳

lkmill marked this pull request as draft February 22, 2021 10:13

[New] Use import() on esm files in supported node versions

28d6e51

ljharb reviewed Feb 23, 2021

View reviewed changes

bin/load-module-esm-support.js Outdated Show resolved Hide resolved

bin/tape Outdated Show resolved Hide resolved

ljharb reviewed Feb 25, 2021

View reviewed changes

bin/load-module-esm-support.js Outdated Show resolved Hide resolved

ljharb reviewed Feb 25, 2021

View reviewed changes

bin/load-module-esm-support.js Outdated Show resolved Hide resolved

coreyfarrell reviewed Feb 25, 2021

View reviewed changes

bin/load-module-esm-support.js Outdated Show resolved Hide resolved

bin/load-module-esm-support.js Outdated Show resolved Hide resolved

bin/load-module-esm-support.js Outdated Show resolved Hide resolved

bin/tape Outdated Show resolved Hide resolved

ljharb mentioned this pull request Mar 3, 2021

tape runner does not support ESM files #514

Closed

mourner mentioned this pull request Apr 16, 2021

D3 packages are incompatible with Node projects that use "type": "module" d3/d3#3469

Closed

haroun suggested changes May 8, 2021

View reviewed changes

test/import.js Outdated Show resolved Hide resolved

test/import.js Outdated Show resolved Hide resolved

bin/tape Outdated Show resolved Hide resolved

lkmill commented May 11, 2021

View reviewed changes

lib/results.js Outdated Show resolved Hide resolved

lkmill marked this pull request as ready for review May 11, 2021 13:58

lkmill mentioned this pull request May 13, 2021

Support ECMAScript modules (ESM) cosmiconfig/cosmiconfig#251

Closed

ljharb reviewed May 31, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

lkmill commented May 31, 2021

View reviewed changes

ljharb reviewed Jun 1, 2021

View reviewed changes

bin/import-or-require.js Outdated Show resolved Hide resolved

bin/tape Outdated Show resolved Hide resolved

bin/tape Outdated Show resolved Hide resolved

bin/tape Outdated Show resolved Hide resolved

lib/results.js Outdated Show resolved Hide resolved

ljharb reviewed Jun 1, 2021

View reviewed changes

ljharb requested changes Jun 2, 2021

View reviewed changes

ljharb reviewed Jun 2, 2021

View reviewed changes

bin/tape Outdated Show resolved Hide resolved

bin/tape Outdated Show resolved Hide resolved

index.js Outdated Show resolved Hide resolved

ljharb reviewed Jun 2, 2021

View reviewed changes

ljharb reviewed Jun 3, 2021

View reviewed changes

.eslintrc Show resolved Hide resolved

bin/tape Outdated Show resolved Hide resolved

bin/tape Outdated Show resolved Hide resolved

ljharb requested changes Jun 4, 2021

View reviewed changes

bin/tape Outdated Show resolved Hide resolved

.eslintrc Show resolved Hide resolved

[Tests] uncaught exceptions and unhandled rejections importing files …

e6d2faf

…with bin/tape

lkmill requested a review from ljharb July 6, 2021 10:44

jimmywarting mentioned this pull request Jul 9, 2021

convert to ESM webtorrent/webtorrent#2096

Closed

ljharb approved these changes Jul 23, 2021

View reviewed changes

.eslintrc Show resolved Hide resolved

ljharb merged commit e6d2faf into tape-testing:master Jul 23, 2021

ljharb mentioned this pull request Jul 26, 2021

author tape itself as ES Modules #414

Closed

ThaUnknown mentioned this pull request Aug 21, 2021

Switch to ESM webtorrent/webtorrent#2177

Closed

27 tasks

jonkoops mentioned this pull request May 10, 2023

Reviewing nodejs resource server example keycloak/keycloak-quickstarts#425

Merged

Use import() on esm files in supported node versions #547

Use import() on esm files in supported node versions #547

Conversation

lkmill commented Feb 22, 2021 • edited Loading

Questions / Caveats

codecov-io commented Feb 22, 2021 • edited Loading

Codecov Report

ljharb left a comment

Choose a reason for hiding this comment

ljharb commented Feb 23, 2021

lkmill commented Feb 23, 2021 • edited Loading

ljharb commented Feb 23, 2021

lkmill commented Feb 23, 2021

ljharb commented Feb 23, 2021

ljharb commented Feb 23, 2021

haroun left a comment

Choose a reason for hiding this comment

lkmill commented May 11, 2021 • edited Loading

lkmill commented May 11, 2021

lkmill commented May 13, 2021

ljharb commented May 13, 2021

codecov-commenter commented May 13, 2021 • edited Loading

Codecov Report

ljharb commented May 14, 2021

lkmill commented May 14, 2021

lkmill commented May 30, 2021

lkmill commented May 30, 2021 • edited Loading

ljharb left a comment

Choose a reason for hiding this comment

lkmill left a comment

Choose a reason for hiding this comment

ljharb commented May 31, 2021

ljharb left a comment

Choose a reason for hiding this comment

lkmill commented Jun 1, 2021

ljharb commented Jun 2, 2021

lkmill commented Jun 2, 2021

ljharb left a comment

Choose a reason for hiding this comment

ljharb Jun 2, 2021

Choose a reason for hiding this comment

ljharb Jun 2, 2021

Choose a reason for hiding this comment

lkmill Jun 2, 2021

Choose a reason for hiding this comment

ljharb left a comment

Choose a reason for hiding this comment

lkmill commented Jun 6, 2021 • edited Loading

ljharb commented Jun 7, 2021

lkmill commented Jun 10, 2021 • edited Loading

ljharb commented Jun 13, 2021

lkmill commented Jun 16, 2021 • edited Loading

lkmill commented Jul 6, 2021

ljharb commented Jul 23, 2021

ljharb commented Jul 26, 2021

ghost commented Sep 20, 2021

lkmill commented Feb 22, 2021 •

edited

Loading

codecov-io commented Feb 22, 2021 •

edited

Loading

lkmill commented Feb 23, 2021 •

edited

Loading

lkmill commented May 11, 2021 •

edited

Loading

codecov-commenter commented May 13, 2021 •

edited

Loading

lkmill commented May 30, 2021 •

edited

Loading

lkmill commented Jun 6, 2021 •

edited

Loading

lkmill commented Jun 10, 2021 •

edited

Loading

lkmill commented Jun 16, 2021 •

edited

Loading