Fix Node.js 4 support with Proxy#5489
Conversation
|
Right now Browserslist 3.0 release is blocked by this issue. Babel 7 needs Browserslist 3.0 too :). “global village” :D. |
| delete real[name]; | ||
| delete lookup[name]; | ||
| if (typeof Proxy === 'undefined') { | ||
| return real; |
There was a problem hiding this comment.
real is an empty object here, since it's just created based on the prototype. A better fix would be to do, just after the signature of the method:
if (typeof Proxy === 'undefined') {
return deepCyclicCopy(process.env);
}There was a problem hiding this comment.
Sure. Wait a second. Do you have some deepCyclicCopy method already in the project?
Codecov Report
@@ Coverage Diff @@
## master #5489 +/- ##
=======================================
Coverage 61.66% 61.66%
=======================================
Files 213 213
Lines 7069 7069
Branches 3 4 +1
=======================================
Hits 4359 4359
Misses 2709 2709
Partials 1 1
Continue to review full report at Codecov.
|
|
@mjesun done |
|
Cool, but you can discard the other commit 😉 |
|
@mjesun I merged commits (you can do the same with “Squash me Merge” option on GitHub pull requests). |
|
What i meant is that you just need the three lines I added in the review; the rest can be removed 🙂 |
|
@mjesun :D cleaned up |
|
@thymikee ... and the |
|
Thanks :). Waiting for release =^_^=. |
|
@mjesun could you please release it. Browserslist is still broken. |
|
Needs #5496 before release |
|
Let me do a patch with the current status :) |
|
22.2.2 is out! |
|
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 many projects must support Node.js 4 until LTS end (August 2018) we decided for silent support of Node.js. For example, big projects like Autoprefixer and PostCSS must support Node.js 4, so we run tests on Travis CI with Node.js 4.
Proxyis used for a small win32 fix. Maybe it could be fixed even without Proxy, but I made a smallifto be clean.Test plan
Since it is silent support of Node.js 4 I didn’t add tests.