Other things that may be worth fixing, marked with FIXME

These are not AS3 regressions.

Author: glasser

docs/source/api/apollo-server.md

@@ -5,6 +5,7 @@ api_reference: true
 ---
 
 This API reference documents the exports from the `apollo-server` package.
+FIXME no it doesn't, it's a weird mishmosh of apollo-server and the integrations. We should refactor it somehow.
 
 ## `class ApolloServer`
 

docs/source/data/data-sources.mdx

@@ -30,7 +30,7 @@ flowchart LR;
 
 ## Open-source implementations
 
-All data source implementations extend the generic [`DataSource` abstract class](https://github.com/apollographql/apollo-server/blob/main/packages/apollo-datasource/src/index.ts), which is included in the `apollo-server` library. Subclasses define whatever logic is required to communicate with a particular store or API.
+All data source implementations extend the generic [`DataSource` abstract class](https://github.com/apollographql/apollo-server/blob/main/packages/apollo-datasource/src/index.ts), which is included in the `apollo-server` library. FIXME it's in the `apollo-server` repo but it's in its own `apollo-datasource` package! Subclasses define whatever logic is required to communicate with a particular store or API.
 
 Apollo and the larger community maintain the following open-source implementatons:
 

docs/source/data/resolvers.mdx

@@ -164,7 +164,7 @@ Note that you can define your resolvers across as many different files and objec
 
 ## Resolver chains
 
-Whenever a query asks for a field that contains an object type, the query _also_ asks for _at least one field_ of that object (if it didn't, there would be no reason to include the object in the query). A query always "bottoms out" on fields that contain either a scalar or a list of scalars.
+Whenever a query asks for a field that contains an object type, the query _also_ asks for _at least one field_ of that object (if it didn't, there would be no reason to include the object in the query). A query always "bottoms out" on fields that contain either a scalar or a list of scalars. FIXME technically can also be things like "list of list of scalars", not to mention non-nullable, and also there are enums which aren't technically scalars.
 
 Therefore, whenever Apollo Server _resolves_ a field that contains an object type, it always then resolves one or more fields of that object. Those subfields might in turn _also_ contain object types. Depending on your schema, this object-field pattern can continue to an arbitrary depth, creating what's called a **resolver chain**.
 
@@ -388,7 +388,7 @@ const server = new ApolloServer({
 }
 ```
 
-> The fields of the object passed to your `context` function differ if you're using middleware besides Express. [See the API reference for details.](/api/apollo-server/#middleware-specific-context-fields)
+> The fields of the object passed to your `context` function differ if you're using middleware besides Express. [See the API reference for details.](/api/apollo-server/#middleware-specific-context-fields) FIXME it's not really clear here that `apollo-server` wraps Express and so it gets the Express context args. Also we've barely mentioned middleware so far in the docs.
 
 Context initialization can be asynchronous, allowing database connections and other operations to complete:
 
@@ -411,7 +411,7 @@ A resolver function's return value is treated differently by Apollo Server depen
 |---|---|
 | Scalar / object | <p>A resolver can return a single value or an object, as shown in [Defining a resolver](#defining-a-resolver). This return value is passed down to any nested resolvers via the `parent` argument.</p> |
 | `Array` | <p>Return an array if and only if your schema indicates that the resolver's associated field contains a list.</p><p>After you return an array, Apollo Server executes nested resolvers for each item in the array. </p>|
-| `null` / `undefined` | <p>Indicates that the value for the field could not be found.</p> <p>If your schema indicates that this resolver's field is nullable, then the operation result has a `null` value at the field's position.</p><p>If this resolver's field is _not_ nullable, Apollo Server sets the field's _parent_ to `null`. If necessary, this process continues up the resolver chain until it reaches a field that _is_ nullable. This ensures that a response never includes a `null` value for a non-nullable field.</p>|
+| `null` / `undefined` | <p>Indicates that the value for the field could not be found.</p> <p>If your schema indicates that this resolver's field is nullable, then the operation result has a `null` value at the field's position.</p><p>If this resolver's field is _not_ nullable, Apollo Server sets the field's _parent_ to `null`. If necessary, this process continues up the resolver chain until it reaches a field that _is_ nullable. This ensures that a response never includes a `null` value for a non-nullable field. FIXME this should mention that if this happens, an error is returned too!</p>|
 | [`Promise`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises) | <p>Resolvers often perform asynchronous actions, such as fetching from a database or back-end API. To support this, a resolver can return a promise that resolves to any other supported return type.</p> |
 
 

docs/source/integrations/middleware.md

@@ -4,6 +4,9 @@ sidebar_title: Node.js middleware
 description: Use Apollo Server with Express, Koa, and more
 ---
 
+FIXME the way we document the packages needs some work in general. first of all the idea that there are multiple packages at all isn't really introduced early in the docs. secondly, it's not always applyMiddleware, there's createHandler too. we should actually document all the middlewares!!! or give up and explicitly only care about the READMEs. Not an AS3 regression though. Also we shouldn't refer to `apollo-server` as the "core" package since it's not `apollo-server-core`. I like "batteries-included".
+Another alternative is that we should endeavour to document Express explicitly and basically say "for the others, figure it out yourself" (or README only)
+
 Apollo Server integrates easily with several popular Node.js middleware libraries.
 To integrate, first install the appropriate package from the table below _instead of_
 the core `apollo-server` package:

docs/source/monitoring/health-checks.md

@@ -8,6 +8,7 @@ Health checks are often used by load balancers to determine if a server is avail
 This basic health check may not be comprehensive enough for some applications and depending on individual circumstances, it may be beneficial to provide a more thorough implementation by defining an `onHealthCheck` function to the `ApolloServer` constructor options.  If defined, this `onHealthCheck` function should return a `Promise` which _rejects_ if there is an error, or _resolves_ if the server is deemed _ready_.  A `Promise` _rejection_ will result in an HTTP status code of 503, and a _resolution_ will result in an HTTP status code of 200, which is generally desired by most health-check tooling (e.g. Kubernetes, AWS, etc.).
 
 > **Note:** Alternatively, the `onHealthCheck` can be defined as an `async` function which `throw`s if it encounters an error and returns when conditions are considered normal.
+FIXME we should just document this as being an async function and use an async function in the example
 
 ```js{10-19}
 const { ApolloServer, gql } = require('apollo-server');

docs/source/performance/caching.md

@@ -4,6 +4,8 @@ sidebar_title: Caching
 description: Configure caching behavior on a per-field basis
 ---
 
+FIXME should note that federation doesn't support caching well though maybe link to something that explains workarounds?
+
 Apollo Server enables you to define cache control settings (`maxAge` and `scope`) for each field in your schema:
 
 ```graphql{5,7}
@@ -214,7 +216,7 @@ If you run Apollo Server behind a CDN or another caching proxy, you can configur
 
 Because CDNs and caching proxies only cache GET requests (not POST requests, which Apollo Client sends for all operations by default), we recommend enabling [automatic persisted queries](./apq/) and the [`useGETForHashedQueries` option](./apq/) in Apollo Client.
 
-Alternatively, you can set the `useGETForQueries` option of [HttpLink](https://www.apollographql.com/docs/react/api/link/apollo-link-http) in your `ApolloClient` instance, but **this is less secure** because your query string and GraphQL variables are sent as plaintext URL query parameters.
+Alternatively, you can set the `useGETForQueries` option of [HttpLink](https://www.apollographql.com/docs/react/api/link/apollo-link-http) in your `ApolloClient` instance, but **this may be less secure** because your query string and GraphQL variables are sent as plaintext URL query parameters which are more likely to be saved in logs by some server or proxy between the user and your GraphQL server.  FIXME I tried to improve this but it still doesn't make that much sense because `useGETForHashedQueries` certainly puts variables in the URL... I think the real reason to avoid useGETForQueries is that GETs have a length limit!
 
 ### Disabling `Cache-Control`
 

docs/source/proxy-configuration.md

@@ -5,6 +5,8 @@ description: Configuring proxy settings for outgoing requests
 
 Certain features of the Apollo platform require Apollo Server to make outgoing requests to Apollo Studio.  These include:
 
+FIXME: could mention usage reporting and schema reporting too, though for them should just override fetcher...
+
 * Managed federation
 * Operation registry
 

docs/source/schema/custom-scalars.md

@@ -12,7 +12,7 @@ To define a custom scalar, add it to your schema like so:
 scalar MyCustomScalar
 ```
 
-Object types in your schema can now contain fields of type `MyCustomScalar`. However, Apollo Server still needs to know how to interact with values of this new scalar type.
+Object types in your schema can now contain fields of type `MyCustomScalar`. (FIXME: and also it can be an argument or input type field!) However, Apollo Server still needs to know how to interact with values of this new scalar type.
 
 ## Defining custom scalar logic
 
@@ -70,13 +70,14 @@ In the example above, the `Date` scalar is represented on the backend by the `Da
 
 ### `parseValue`
 
-The `parseValue` method converts the scalar's `serialize`d JSON value to its back-end representation before it's added to a resolver's `args`.
+The `parseValue` method converts the scalar's `serialize`d JSON value to its back-end representation before it's added to a resolver's `args`. FIXME it's not really the serialized value: serialize always outputs strings, but this can be any JSON value.
 
 Apollo Server calls this method when the scalar is provided by a client as a [GraphQL variable](https://graphql.org/learn/queries/#variables) for an argument. (When a scalar is provided as a hard-coded argument in the operation string, [`parseLiteral`](#parseliteral) is called instead.)
 
 ### `parseLiteral`
 
 When an incoming query string includes the scalar as a hard-coded argument value, that value is part of the query document's abstract syntax tree (AST). Apollo Server calls the `parseLiteral` method to convert the value's AST representation (which is always a string) to the scalar's back-end representation.
+(FIXME re "always a string", sorta? you can use other AST node types like int values (like you show above?), it's just that the int value node does internally store its int value as a string. but this kinda makes it sound like scalars *must* always be provided as things wrapped in double quotes?)
 
 In [the example above](#example-the-date-scalar), `parseLiteral` converts the AST value from a string to an integer, and _then_ converts from integer to `Date` to match the result of `parseValue`.
 
@@ -163,6 +164,9 @@ server.listen().then(({ url }) => {
 });
 ```
 
+FIXME: A few concerns with this example. (a) It doesn't actually run: no `Query` defined, but no indicator that this is a partial example. (b) No resolver for the oddValue field. (c) It doesn't explain what "can only contain odd integers means" and the answer is actually kinda strange: if a field would return something that is not an odd number then it just gets converted to `null` in the output, and ditto in input, but there's no errors. (d) This "null" is basically "the way we represent this non-null value in JSON"; if you have a field declared as `Odd!` (input or output) and you put an even number there, it'll happily turn it into `null` *with no error despite the `!`*! (e) You can put floating-point odd numbers and it'll work too. (f) Also you can put a string containing an odd number and it works.
+I changed oddValue above to throw instead of return null, which helps with c and d.
+
 ## Importing a third-party custom scalar
 
 If another library defines a custom scalar, you can import it and use it just like any other symbol.

docs/source/schema/directives.md

@@ -25,6 +25,7 @@ This example shows the `@deprecated` directive, which is a [default directive](#
 Each directive can only appear in _certain_ locations within a GraphQL schema or operation. These locations are listed in the directive's definition.
 
 For example, here's the GraphQL spec's definition of the `@deprecated` directive:
+FIXME I believe this is now outdated as `@deprecated` can appear in more places now.
 
 ```graphql
 directive @deprecated(