-
-
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
Fix Node.js 4 support #5142
Fix Node.js 4 support #5142
Conversation
We could just transpile rest/spread and avoid the issue entirely... |
@SimenB yeap, some packages transpile rest operator (seems like transpile by Flow) and some doesn’t. I am not sure how to enable transpiling for the whole project. |
Should be enough to add https://www.npmjs.com/package/babel-plugin-transform-es2015-spread to https://github.com/facebook/jest/blob/1521c516c2cae8f4bfe41605640f273f9c17fb87/.babelrc Right now we transpile rest arguments, but not spread, it seems |
Thanks. Tomorrow I will update PR. |
Codecov Report
@@ Coverage Diff @@
## master #5142 +/- ##
==========================================
- Coverage 60.7% 60.66% -0.05%
==========================================
Files 201 201
Lines 6691 6693 +2
Branches 4 4
==========================================
- Hits 4062 4060 -2
- Misses 2628 2632 +4
Partials 1 1
Continue to review full report at Codecov.
|
packages/jest-jasmine2/src/index.js
Outdated
@@ -52,7 +52,7 @@ async function jasmine2( | |||
const originalIt = environment.global.it; | |||
environment.global.it = (...args) => { | |||
const stack = callsites()[1]; | |||
const it = originalIt(...args); | |||
const it = originalIt.apply(this, ...args); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need the apply
, do we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. fix was pushed
I added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense.
It doesn't hurt to transpile this, even though "officially" we don't have to
Thanks! I’m not gonna be in front of a computer soon, maybe @mjesun can make a patch release? |
Ping 😊 |
Hi! Jest 22.0.4 should be out. Enjoy! |
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
Improve unofficial Node.js 4 support
Test plan
No tests since support is unofficial