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

Consider warning/error if "Disabled" plugin combined with explicit installation #7099

Closed
glasser opened this issue Oct 31, 2022 · 0 comments · Fixed by #7104
Closed

Consider warning/error if "Disabled" plugin combined with explicit installation #7099

glasser opened this issue Oct 31, 2022 · 0 comments · Fixed by #7104

Comments

@glasser
Copy link
Member

glasser commented Oct 31, 2022

We received reports of users confused that the effect of (essentially):

new ApolloServer({
  plugins: [
    ApolloServerPluginUsageReporting({ some: 'custom configuration' }),
    ApolloServerPluginUsageReportingDisabled(),
  ],
})

is to leave usage reporting on despite the "disabled" plugin. We should consider just making this into an error (which would be sorta-backwards-incompatible but only sorta, this was not intended to have well-defined semantics, and AS4 is new anyway), or perhaps a warning although warnings easily just become spam.

(The "disabled" plugins actually mean "disable the automatic installation of the corresponding plugin with default configuration" but currently has no effect if you manually install the corresponding plugin.)

glasser added a commit that referenced this issue Oct 31, 2022
- Make it an error to install a built-in plugin and its "Disabled"
  counterpart in the same server. "Disabled" really means "don't install
  by default", but it's reasonable to assume it would counteract a
  manual installation of the same plugin too.  For simplicity, this
  makes installing both flavors into an error at start time. This is
  technically backwards-incompatible but we are still early in the ASv4
  lifecycle and this combination wasn't expected to have well-defined
  behavior.
- Add `ApolloServerPluginSchemaReportingDisabled` which allows you to
  override the effect of the `APOLLO_SCHEMA_REPORTING` environment
  variable.
- Doc improvements.

Fixes #4778. Fixes #7099.
glasser added a commit that referenced this issue Oct 31, 2022
- Make it an error to install a built-in plugin and its "Disabled"
  counterpart in the same server. "Disabled" really means "don't install
  by default", but it's reasonable to assume it would counteract a
  manual installation of the same plugin too.  For simplicity, this
  makes installing both flavors into an error at start time. This is
  technically backwards-incompatible but we are still early in the ASv4
  lifecycle and this combination wasn't expected to have well-defined
  behavior.
- Add `ApolloServerPluginSchemaReportingDisabled` which allows you to
  override the effect of the `APOLLO_SCHEMA_REPORTING` environment
  variable.
- Doc improvements.

Fixes #4778. Fixes #7099.
glasser added a commit that referenced this issue Oct 31, 2022
- Make it an error to install a built-in plugin and its "Disabled"
counterpart in the same server. "Disabled" really means "don't install
by default", but it's reasonable to assume it would counteract a manual
installation of the same plugin too. For simplicity, this makes
installing both flavors into an error at start time. This is technically
backwards-incompatible but we are still early in the ASv4 lifecycle and
this combination wasn't expected to have well-defined behavior.
- Add `ApolloServerPluginSchemaReportingDisabled` which allows you to
override the effect of the `APOLLO_SCHEMA_REPORTING` environment
variable.
- Doc improvements.

Fixes #4778. Fixes #7099.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 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 a pull request may close this issue.

1 participant