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

Upgrade to graphql-upload v8, dropping upload support for Node.js v6. #2054

Merged
merged 11 commits into from Dec 4, 2018

Commits on Nov 30, 2018

  1. Configuration menu
    Copy the full SHA
    0c2fbc3 View commit details
    Browse the repository at this point in the history
  2. 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.
    abernix committed Nov 30, 2018
    Configuration menu
    Copy the full SHA
    9c563fa View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    7e6d6cf View commit details
    Browse the repository at this point in the history
  4. 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.
    abernix committed Nov 30, 2018
    Configuration menu
    Copy the full SHA
    2fd56f0 View commit details
    Browse the repository at this point in the history
  5. (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.
    abernix committed Nov 30, 2018
    Configuration menu
    Copy the full SHA
    6dc132e View commit details
    Browse the repository at this point in the history
  6. (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.).
    abernix committed Nov 30, 2018
    Configuration menu
    Copy the full SHA
    cd6e575 View commit details
    Browse the repository at this point in the history
  7. (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!
    abernix committed Nov 30, 2018
    Configuration menu
    Copy the full SHA
    705256e View commit details
    Browse the repository at this point in the history

Commits on Dec 3, 2018

  1. Update CHANGELOG.md

    abernix committed Dec 3, 2018
    Configuration menu
    Copy the full SHA
    8a58e9f View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    363472c View commit details
    Browse the repository at this point in the history
  3. Update ApolloServer.ts

    abernix committed Dec 3, 2018
    Configuration menu
    Copy the full SHA
    fe2d597 View commit details
    Browse the repository at this point in the history

Commits on Dec 4, 2018

  1. Configuration menu
    Copy the full SHA
    abb8dc5 View commit details
    Browse the repository at this point in the history