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
Upgrade to graphql-upload v8, dropping upload support for Node.js v6. #2054
Upgrade to graphql-upload v8, dropping upload support for Node.js v6. #2054
Commits on Nov 30, 2018
-
Configuration menu - View commit details
-
Copy full SHA for 0c2fbc3 - Browse repository at this point
Copy the full SHA 0c2fbc3View commit details -
Throw error at startup when file uploads are enabled on Node.js < 8.5.0.
Due to changes in the third-party `graphql-upload` package which Apollo Server utilizes to implement out-of-the-box file upload functionality, we must drop support for file uploads in versions of the Node.js engine prior to v8.5.0. Since file uploads are supported by default in Apollo Server 2.x, and there is an explicit dependency on `graphql-upload`, we must prevent users who are affected by this mid-major-release deprecation by being surprised by the sudden lack of upload support. By `throw`-ing an error at server startup for affected users, we certainly are breaking a semantic versioning agreement for these users, however with a relatively simple ergonomic (setting `uploads: false`) we allow those users who are NOT utilizing file uploads (as we believe is the case with a majority) to continue using their version of Node.js until it reaches the end of its supported lifetime (as dictated by its Long Term Support agreement with the Node.js Foundation). If we did not `throw` the error at server start-up, those affected may not notice since they may update and start their updated server without noticing the impending chance of failure when someone tries updating! Apollo Server 2.x has attempted to maintain full compatibility with versions of Node.js which are still under Long Term Support agreements with the Node.js Foundation. While this continues to mostly be true, file uploads are an exception which we've now had to make. Third-party open-source projects must absolutely do what's best for their project. From an architecture standpoint, I suspect that we (the designers behind Apollo Server) are mostly to blame for this. Namely, it's unfortunate that we had made such an incredibly coupled integration with a third-party package that we restricted our users from incrementally adopting the changes (and new/improved functionality) of, in this particular case, the `graphql-upload` package. I hope we can take better care with decisions like this in the future! Lastly, this commit also adds documentation to help those affected.
Configuration menu - View commit details
-
Copy full SHA for 9c563fa - Browse repository at this point
Copy the full SHA 9c563faView commit details -
Configuration menu - View commit details
-
Copy full SHA for 7e6d6cf - Browse repository at this point
Copy the full SHA 7e6d6cfView commit details -
Don't enable uploads when using an unsupported Node.js in tests.
Now that there are specific versions of Node.js which don't support file uploads (namely, <= 8.5.0) we need to explicitly disable uploads on those versions, similar to how those users must opt-in to that behavior by setting `uploads: false` in their Apollo Server constructor options. This effectively accomplishes exactly that, but only when necessary.
Configuration menu - View commit details
-
Copy full SHA for 2fd56f0 - Browse repository at this point
Copy the full SHA 2fd56f0View commit details -
(tests) Switch to a very explicit Node.js 10 restriction.
I hope to actually remove this limitation, but I'm going to iterate on it first. This also just switches to a `NODE_MAJOR_VERSION` constant from the `apollo-server-integration-testsuite` rather than using the GTE function on the same input since...math.
Configuration menu - View commit details
-
Copy full SHA for 6dc132e - Browse repository at this point
Copy the full SHA 6dc132eView commit details -
(tests) Skip file upload tests for
NODE_MAJOR_VERSION === 6
.This commit looks way more complicated than it really is thanks to Prettier trying to be helpful. All I've done is add `NODE_MAJOR_VERSION === 6` as a version NOT to test uploads for, in an effort to fix the failing tests (failing appropriately since Node.js 6 with file uploads throws an error right now and cannot run uploads anymore.).
Configuration menu - View commit details
-
Copy full SHA for cd6e575 - Browse repository at this point
Copy the full SHA cd6e575View commit details -
(tests): Re-enable file upload tests for Node.js 10. 🎉🎉🎉
This commit again looks quite complicated, but's merely an over-complication inflicted by Prettification. Disabling whitespace differences when viewing this commit, the functional change here is that we no longer skip many file upload tests when the (semver) major segment of `process.version` is `10`. I couldn't be happier to get rid of this exception for file upload tests on Node.js 10, which was an unfortunate reality of the non-updated `graphql-upload` module world we previously lived in. Thanks, @jaydenseric, for the newfound (to us!) Node.js upload support!
Configuration menu - View commit details
-
Copy full SHA for 705256e - Browse repository at this point
Copy the full SHA 705256eView commit details
Commits on Dec 3, 2018
-
Configuration menu - View commit details
-
Copy full SHA for 8a58e9f - Browse repository at this point
Copy the full SHA 8a58e9fView commit details -
Configuration menu - View commit details
-
Copy full SHA for 363472c - Browse repository at this point
Copy the full SHA 363472cView commit details -
Configuration menu - View commit details
-
Copy full SHA for fe2d597 - Browse repository at this point
Copy the full SHA fe2d597View commit details
Commits on Dec 4, 2018
-
Configuration menu - View commit details
-
Copy full SHA for abb8dc5 - Browse repository at this point
Copy the full SHA abb8dc5View commit details