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

Release 2.4.0 #2215

Merged
merged 31 commits into from
Feb 7, 2019
Merged

Release 2.4.0 #2215

merged 31 commits into from
Feb 7, 2019

Conversation

abernix
Copy link
Member

@abernix abernix commented Jan 23, 2019

This is a PR tracking a release-x.y.z branch for an upcoming semver minor release of Apollo Server: v2.4.0 (i.e. release-2.4.0). The intention here is to keep the master branch a bit more flexible while landing things which have been decidedly put into a release on this more specific release branch. PRs for 2.4.0 features can be targeted toward this release-2.4.0 branch (and thus appear in this PR's history), in addition to being merged into the master branch and providing the ability to backport fixes on the 2.3.x line from there.

We should experiment with this pattern a bit more in the future but my hope is that it will provide more visibility into what's already staged for a particular release and, more importantly, clarity into the npm "pre" (e.g. alpha, beta, next) releases that have been cut off it for those who are interested in testing/following/providing feedback on those releases, once the original PRs that introduce those features have landed.

There have already been 2.4.0 alpha releases cut from release-2.4.0 (for example, the alpha releases on #2111 — which is now merged into this release-2.4.0 branch), but I'd like to merge @rkorrelboom's work which introduces a new Fastify integration from #1971, so that'll be next in line for a pre-release!

Check the appropriate milestone for more details on what's slated for 2.4.0.

… use.

Without this change, the `document` property was not set on the
`requestContext` for consumption by request pipeline plugins.

To further guard against this oversight, I've removed the extra `document`
variable which was being used as scoped state for the document and switched to
directly using (and assigning to) the `requestContext.document`.

Nice catch, @glasser!

Ref: https://github.com/apollographql/apollo-server/pull/2111/files#r247617469
The parsed/validated cache store is on by default.  While it could be disabled,
in theory, it cannot be disabled since its an internal property of the
request pipeline processor class.

See confusion here:

  withspectrum/spectrum#4533 (comment)
…ore`.

While the implementation of the `documentStore` is currently simple enough
to never throw (it is in-memory), it makes some sense to guard against future
extended functionality where an exception might be raised.

Since storing this object in a distributed memory store isn't currently
feasible, I'm not sure what such an exception would be right now, but I
don't mind being proactive!

Ref: https://github.com/apollographql/apollo-server/pull/2111/files#r247618501
Previously, this used the `JSON.stringify` length, but this is slightly more
aware.
…class.

The implementation of object-size approximation which is used for cache
eviction purposes in the `InMemoryLRUCache` implementation (via `lru-cache`)
was a short-term location for extensible logic which is better located
within `ApolloServerBase`.

This is particularly important since future logic may necessitate knowing or
understanding the current size (roughly, memory usage) of the in-memory
storage.  Effective immediately, this adds support for providing a `dispose`
function which is called when an object is purged from the cache to make
room for another.
In an effort to see how effective this cache is in production during this
alpha phase, we'll print out the stats on the document store every 60
seconds.
… seconds."

This reverts commit 7a0d0e6, as I intended
when I originally introduced it.

Ref: #2111 (comment)
…dated

Cache successfully parsed and validated documents for future requests.
rkorrelboom and others added 4 commits January 23, 2019 15:43
* feat(fastify) Apollo Fastify server integration resolve #626

* feat(fastify) Use createHandler instead of applyMiddleware #626

* feat(fastify) Fix integration test for node 10 #626

* feat(fastify) Update README's with fastify createHandler interface #626

* feat(fastify) Implement the fastify createHandler as a synchronous method #626

* (fastify) Tweaks to re-align with the parallel work in #2054.

* (fastify): Use port 9999 rather than 8888 for tests.  Because Gatsby.

This specific port per integration is pretty brittle to begin with, but it
does work.  Currently, the fact that it works is facilitated by the fact
that most people don't use 5555 (Hapi) and 6666 (Express) for anything.

That said, the ever-popular Gatsby uses 8888 by default, so let's use 9999!

* (fastify) Remove duplicative assertion in upload initialization.

* (fastify) Implement fastify upload middleware

* (fastify) Fix linting issues

* (fastify) Update package-lock
This will cause the first release of `apollo-server-fastify` to land at
`2.4.0-alpha.3`.  (Hopefully.)
@iagomelanias
Copy link

iagomelanias commented Jan 23, 2019

apollo-server-fastify is already taken on NPM. I believe we will need to use another name and update the documentation. ☹️

@abernix abernix self-assigned this Jan 23, 2019
@abernix
Copy link
Member Author

abernix commented Jan 23, 2019

@iagomelanias I noted that on the other PR just a moment ago (#1971 (comment)). Hopefully we can figure something out, but it might be worth waiting a few days to see how it pans out.

For the time being, 2.4.0-alpha.3 has been published, though Fastify was not included in that publishing due to the lack of package ownership.

@nahtnam
Copy link

nahtnam commented Feb 1, 2019

The package hasn't been updated for a year (https://www.npmjs.com/package/apollo-server-fastify). Maybe I can reach out to the developer and ask them if they are willing to release it? In which case we could just bump it to 2.0 and announce that its a breaking change.

abernix and others added 7 commits February 4, 2019 17:22
#2217)

… version

This PR should hopefully correct some typing issues we are currently facing.

The problem:
- this package references v5.0.1 for @types/koa-bodyparser
- when we start our server that also uses koa-bodyparser for non-graphql routes, the type for koa.Request sets the "body" field to:

```
declare module "koa" {
    interface Request {
        body: {} | null | undefined;
        rawBody: {} | null | undefined;
    }
}
```

Which breaks our type defs for koa-bodyparser as we actually want "body" to be set to "any."

The v5.0.1 that was originally placed here is no longer "latest" and they have published a new v4 version that has the correct typing for koa-bodyparser (setting body to any). - https://www.npmjs.com/package/@types/koa-bodyparser

<!--
  Thanks for filing a pull request on GraphQL Server!

  Please look at the following checklist to ensure that your PR
  can be accepted quickly:
-->

TODO:

* [ ] Update CHANGELOG.md with your change (include reference to issue & this PR)
* [x] Make sure all of the significant new logic is covered by tests
* [x] Rebase your changes on master so that they can be merged easily
* [x] Make sure all tests and linter rules pass
…1971)"

This TEMPORARILY reverts commit 069110b,
which was the result of the work done in #1971 by @rkorrelboom.

Unfortunately, we need to put this on ice while we wait for movement on a
package naming conflict.  The dialog surrounding this is under way, as
I've explained in the PR:

#1971 (comment)

I'm excited to re-land this in an upcoming version, but there's no reason to
block the 2.4.0 release for it right now.

I will open a new PR with the work from #1971 in due time.
@abernix
Copy link
Member Author

abernix commented Feb 7, 2019

Heads up, this is landing without Fastify support.

I don't think we should wait for that to get worked out before we release 2.4.0 with its other fixes and improvements, so I've reverted the merging of that PR into this via e971bdf and opened #2280 which will re-land it when we're ready.

@abernix abernix merged commit c5b71f8 into master Feb 7, 2019
@abernix
Copy link
Member Author

abernix commented Feb 8, 2019

@abernix abernix mentioned this pull request Mar 22, 2019
1 task
@abernix abernix added the 📦 release Applied to PRs which track upcoming releases. label Feb 6, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📦 release Applied to PRs which track upcoming releases.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants