-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
jest-diff: Export as ECMAScript module #8873
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8873 +/- ##
==========================================
- Coverage 64.12% 64.11% -0.01%
==========================================
Files 275 275
Lines 11629 11623 -6
Branches 2845 2845
==========================================
- Hits 7457 7452 -5
Misses 3545 3545
+ Partials 627 626 -1
Continue to review full report at Codecov.
|
Do y’all think it would help or hurt to distinguish versions for breaking change in README:
And also move ECMAScript example above CommonJS example? And identify version of new options and named exports? |
The readme won't be included for jest 24, so I don't think that's needed. |
Wouldn't this be better as a named export instead of default? const diff = require('jest-diff'); // Jest 24 or earlier
const { diff } = require('jest-diff'); // Jest 25 or later |
Either works for me, I'll leave it to @pedrottimark 🙂 It does make it cleaner for CJS, though |
I bring this up because it probably should be standardized across all of jest’s packages. My vote goes to named exports for whatever its worth. |
Thank you for bringing this up. Among others, Nicholas C. Zakas has explained benefits of named versus default export. The only reason why it might not happen before Jest 25 is that I am right now exploring how the public API might grow so a naming convention for the exports will be consistent. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Because a caller might need to construct
Diff
instances to write a custom cleanup algorithm, it must be exported as a class value instead of a type.Breaking change: Because we rewrote as ECMAScript and TypeScript exports, then the class value can be used in type declarations! Thank you, Simen for suggesting this solution ❤️
CommonJS callers rewrite default export:
But see README.md because new named export
diffStringsUnified
might be better for youHowever, if I understand the situation correctly, for TypeScript declarations likediff: Diff
ordiffs: Array<Diff>
when the class is imported across a package boundary, an additional type declaration is needed to avoiderror TS2749: 'Diff' refers to a value, but is being used as a type here.
Corrected and edited
README.md
Did not add toBegan as correction to #8841CHANGELOG.md
because it is an immediateTest plan
diffStringsRaw.test.ts
which verifies exports, includingDiff
as typejest-matcher-utils
also verifies exports, includingDiff
as type