Skip to content
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

chore: Bump typescript to 3.2.4 #25

Merged
merged 2 commits into from
Oct 25, 2019

Conversation

eps1lon
Copy link
Contributor

@eps1lon eps1lon commented Oct 23, 2019

Closes #24

3.1 needed for module scope properties but this breaks eslint typescript parser. Bumping that requires 3.2 so we end up using 3.2

changes in `lib/`
diff --git a/lib/index.d.ts b/lib/index.d.ts
index 88b946c..de35968 100644
--- a/lib/index.d.ts
+++ b/lib/index.d.ts
@@ -1,7 +1,16 @@
-declare const _default: ((expectation: () => void, timeout?: number, interval?: number) => Promise<{}>) & {
+/**
+ * Waits for the expectation to pass and returns a Promise
+ *
+ * @param  expectation  Function  Expectation that has to complete without throwing
+ * @param  timeout  Number  Maximum wait interval, 4500ms by default
+ * @param  interval  Number  Wait-between-retries interval, 50ms by default
+ * @return  Promise  Promise to return a callback result
+ */
+declare const waitForExpect: {
+    (expectation: () => void, timeout?: number, interval?: number): Promise<{}>;
     defaults: {
         timeout: number;
         interval: number;
     };
 };
-export default _default;
+export default waitForExpect;
diff --git a/lib/index.js b/lib/index.js
index 3984921..2a2a6d7 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -55,10 +55,8 @@ var waitForExpect = function waitForExpect(expectation) {
   });
 };
 
-var _default = Object.assign(waitForExpect, {
-  defaults: defaults
-});
-
+waitForExpect.defaults = defaults;
+var _default = waitForExpect;
 exports.default = _default;
 module.exports = exports.default;
 module.exports.default = exports.default;
\ No newline at end of file
diff --git a/lib/toBeInRangeMatcher.d.ts b/lib/toBeInRangeMatcher.d.ts
index 301e8e7..caaf585 100644
--- a/lib/toBeInRangeMatcher.d.ts
+++ b/lib/toBeInRangeMatcher.d.ts
@@ -6,7 +6,7 @@ declare namespace jest {
         }): R;
     }
 }
-declare function toBeInRange<T>(received: number, {min, max}: {
+declare function toBeInRange<T>(received: number, { min, max }: {
     min: number;
     max: number;
 }): {
diff --git a/lib/toBeInRangeMatcher.js b/lib/toBeInRangeMatcher.js
index 70a4c8b..2d2841d 100644
--- a/lib/toBeInRangeMatcher.js
+++ b/lib/toBeInRangeMatcher.js
@@ -3,6 +3,7 @@
 /* eslint-env jest */
 // XXX Not sure how to make eslint not to complain here, but looks like this
 // declaration works, so leaving it for now
+// eslint-disable-next-line no-unused-vars
 function toBeInRange(received, _ref) {
   var min = _ref.min,
       max = _ref.max;

This looks ok and even better than before since we now include the jsdoc block.

3.1 needed for module scope properties but
this breaks eslint typescript parser. Bumping that
requires 3.2 so we end up using 3.2
@lgandecki
Copy link
Member

Thanks! I agree - this does look better to me.
I'm a bit surprised that eslint started complaining in the custom jest matcher. Do you know why? :)

Maybe we should switch to the typescript no-unused rule now:

{
  "rules": {
    // note you must disable the base rule as it can report incorrect errors
    "no-unused-vars": "off",
    "@typescript-eslint/no-unused-vars": 2
  }
}

https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars.md#options

@eps1lon
Copy link
Contributor Author

eps1lon commented Oct 23, 2019

Do you know why? :)

the eslint plugin requires typescript 3.2.4

Maybe we should switch to the typescript no-unused rule now:

Can this be done in a separate PR?

@lgandecki
Copy link
Member

haha, sure, I don't want to be annoying or picky! Usually my understanding is that it's a responsibility of a person creating a PR to not introduce linting ignores unless necessary, but this is minor and I'm happy to fix this myself. I'm already grateful for the work!
Let's wait for Kent to make sure we are aligned and I think we will merge this one.

@eps1lon
Copy link
Contributor Author

eps1lon commented Oct 23, 2019

not introduce linting ignores unless necessary

I understood this as adding new rules sorry. You were suggesting that the new rule would not report this false positive? I'll give it a look.

@lgandecki
Copy link
Member

yeah, the typescript no-unused-vars should be smarter than the default no-unused-vars :)

@eps1lon
Copy link
Contributor Author

eps1lon commented Oct 25, 2019

@lgandecki I switched to rules. I took the liberty and added --report-unused-disable-directives to the linting task which made it clear that we can drop even more eslint-disables.

@lgandecki
Copy link
Member

Thanks a lot, this is great. Since we haven't heard from @kentcdodds I'm assuming he is indifferent so I'm going to merge this one.
Once the semantic-release bots releases this, would you go to the dom-testing-library and make a PR with a bump of the wait-for-expect?

btw on one hand it seems like a chore but I don't think that causes a version bump, so I'll squash as a fix. Thanks for keeping the convention though!

@lgandecki lgandecki merged commit a59b638 into TheBrainFamily:master Oct 25, 2019
@lgandecki
Copy link
Member

🎉 This PR is included in version 3.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@eps1lon eps1lon deleted the chore/bump-ts-31 branch October 25, 2019 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants