Bumps minimum node engine to 8+ #4474
Conversation
flovilmart
commented
Dec 30, 2017
flovilmart
commented
- Adds package-lock
- Adds .nvmrc in order to select proper node version
- Keep deployment / transpilation to node 6
.travis.yml
Outdated
| matrix: | ||
| - MONGODB_VERSION=3.2.13 | ||
| - MONGODB_VERSION=3.4.4 | ||
| - PARSE_SERVER_TEST_DB=postgres | ||
| - PARSE_SERVER_TEST_CACHE=redis | ||
| - NODE_VERSION=8.7 | ||
| - NODE_VERSION=9.3.0 |
There was a problem hiding this comment.
All Node 9.x versions thus far are a complete unusable crap, because their distribution comes with the version of NPM that doesn't work right with Node 9.x. Hopefully, Node 9.4 will finally fix it.
There was a problem hiding this comment.
Seems to be fine, as the tests are passing. The goal of that line is to make sure it’s all forward compatible
|
Why not move entirely into Node 8.x? If we did, we could replace all the promises stuff with |
|
Because steps by steps, and with async await, you’ll probably need to re introduce Babel regenerator to target back 6.x as of today. What you mention is purely a full rewrite that has no direct benefits, and I’m not willing to just do reviews for the next 3 weeks about small refractors. For now, it’s a no go. As for the why node 8, the push adapter leveraging node-apn will require it for http2 in a foreseeable future, as the package is in alpha now. I’m against bumping a min engine version just for the sake of bumping it and rewrites for the sake of using a ‘nicer’ api. Writing code twice don’t make it more maintainable :) |
Codecov Report
@@ Coverage Diff @@
## master #4474 +/- ##
==========================================
+ Coverage 92.65% 92.67% +0.02%
==========================================
Files 119 119
Lines 8659 8659
==========================================
+ Hits 8023 8025 +2
+ Misses 636 634 -2
Continue to review full report at Codecov.
|
flovilmart
changed the title
|
@dplewis I updated it all, we could use that as a baseline, or on a side branch. what do you think? |
I agree with you and I would like to see if there are any performance increase in production. Do you see any issues using this as a baseline? |
|
@dplewis I bumped it , the comment was from december :) |
|
@flovilmart Oops sorry. I keep forgetting Node ages faster than milk. |
|
What do you think? It's reasonable I believe as 6 is not supported anymore. |
|
I think this is reasonable. Lets do it. |
* Use node 8 as development environment * fixup! Use node 8 as development environment * bump node to 8.10 * Targets node 8 for everything * Run npm install so lock file is up to date * Use push adapter v3 * Deflake a test on ParseUser * Adds slight delay after logout * Ensure we wait even if call fails * Use node carbon
