Skip to content

plugins: Declare types rather than use in-line types for life-cycle hooks.#3902

Merged
abernix merged 3 commits into
release-2.12.0from
abernix/explicit-request-listener-types
Mar 26, 2020
Merged

plugins: Declare types rather than use in-line types for life-cycle hooks.#3902
abernix merged 3 commits into
release-2.12.0from
abernix/explicit-request-listener-types

Conversation

@abernix

@abernix abernix commented Mar 19, 2020

Copy link
Copy Markdown
Member

This makes it easier to reason about the types and to keep things DRY, in addition to preserving a more real stacking of types as the stages of server life-cycles progress. E.g., validationDidStart will have a super-set of Required properties which were present in the parsingDidStart phase. Now, rather than actually repeating them verbosely, they will be intersected.

These new explicit types will live in apollo-server-types where they can also be relied upon by @apollo/gateway and other packages, but they'll also be exported directly from the apollo-server-plugin-base package. Reason being: The overall concept of apollo-server-types and this apollo-server-plugin-base package is that they should NOT depend directly on "core", in order to avoid close coupling of plugin support and specific

server versions. In other words, someone should be able to install a version of a plugin that depends on apollo-server-plugin-base in their application which uses a semver compatible plugin API but NOT the same semver range of apollo-server-core, though they are currently similar.

They are duplicated concepts right now where one package is intended to be for public plugin exposure (-plugin-base), while the other (-types) is meant to be used internally. In the future, apollo-server-types and apollo-server-plugin-base will probably roll into the same "types" package, but that is not today!

abernix added 2 commits March 19, 2020 23:24
…ooks.

This makes it easier to reason about the types and to keep things DRY, in
addition to preserving a more real stacking of types as the stages of server
life-cycles progress.  E.g., `validationDidStart` will have a super-set of
`Required` properties which were present in the `parsingDidStart` phase.
Now, rather than actually repeating them verbosely, they will be intersected.

These new explicit types will live in `apollo-server-types` where they can
also be relied upon by `@apollo/gateway` and other packages, but they'll
also be exported directly from the `apollo-server-plugin-base` package.

Reason being: The overall concept of `apollo-server-types` and this
`apollo-server-plugin-base` package is that they should NOT depend directly
on "core", in order to avoid close coupling of plugin support and specific
server versions.  In other words, someone should be able to install a
version of a plugin that depends on `apollo-server-plugin-base` in their
application which uses a semver compatible plugin API but NOT the same
semver range of `apollo-server-core`, though they are currently similar.

They are duplicated concepts right now where one package is intended to be
for public plugin exposure (`-plugin-base`), while the other (`-types`) is
meant to be used internally.  In the future, `apollo-server-types` and
`apollo-server-plugin-base` will probably roll into the same "types"
package, but that is not today!
@abernix abernix requested a review from trevor-scheer March 19, 2020 21:27
Comment thread packages/apollo-server-core/src/requestPipeline.ts
Comment thread packages/apollo-server-core/src/requestPipeline.ts Outdated
Comment thread packages/apollo-server-plugin-base/src/index.ts
@abernix abernix added this to the Release 2.12.0 milestone Mar 23, 2020
@abernix abernix merged commit 7e09cf2 into release-2.12.0 Mar 26, 2020
@abernix abernix deleted the abernix/explicit-request-listener-types branch March 26, 2020 12:39
abernix added a commit to apollographql/federation that referenced this pull request Sep 4, 2020
…llographql/apollo-server#3902)

* plugins: Declare types rather than use in-line types for life-cycle hooks.

This makes it easier to reason about the types and to keep things DRY, in
addition to preserving a more real stacking of types as the stages of server
life-cycles progress.  E.g., `validationDidStart` will have a super-set of
`Required` properties which were present in the `parsingDidStart` phase.
Now, rather than actually repeating them verbosely, they will be intersected.

These new explicit types will live in `apollo-server-types` where they can
also be relied upon by `@apollo/gateway` and other packages, but they'll
also be exported directly from the `apollo-server-plugin-base` package.

Reason being: The overall concept of `apollo-server-types` and this
`apollo-server-plugin-base` package is that they should NOT depend directly
on "core", in order to avoid close coupling of plugin support and specific
server versions.  In other words, someone should be able to install a
version of a plugin that depends on `apollo-server-plugin-base` in their
application which uses a semver compatible plugin API but NOT the same
semver range of `apollo-server-core`, though they are currently similar.

They are duplicated concepts right now where one package is intended to be
for public plugin exposure (`-plugin-base`), while the other (`-types`) is
meant to be used internally.  In the future, `apollo-server-types` and
`apollo-server-plugin-base` will probably roll into the same "types"
package, but that is not today!

* plugins: Same as `source`, `queryHash` is required at `parsingDidStart`.

They are defined at the same time, as noted in the referenced link.

Ref: https://github.com/apollographql/apollo-server/blob/eadcc6b4/packages/apollo-server-core/src/requestPipeline.ts#L196-L197

* Remove unnecessary casting.

Per: https://github.com/apollographql/apollo-server/pull/3902/files#r395767682
Thanks to: d1b7a82
Apollo-Orig-Commit-AS: apollographql/apollo-server@7e09cf2
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants