Skip to content

onDiagnostic callback #648

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

Open
jasonkuhrt opened this issue Nov 20, 2020 · 1 comment
Open

onDiagnostic callback #648

jasonkuhrt opened this issue Nov 20, 2020 · 1 comment
Labels
note/breaking-change This issue existed already type/feat Add a new capability or enhance an existing one

Comments

@jasonkuhrt
Copy link
Contributor

jasonkuhrt commented Nov 20, 2020

Nexus currently and newly has behaviour of:

  • hardcoding the definition of production
  • throwing errors in production
  • warning errors in development

Discussion with @tgriesser led to following proposal:

  • remove hardcoded definition of production from codebase
  • never throw programmer error errors
  • only ever log programmer error errors

Here's my stab at a draft spec

New callback onDiagnostic

onDiagnostic(diagnostic:Diagnostic) {
  // do whatever you want
}

Have an undocumented global function nexusNotify to avoid threading state through the codebase for messaging.

nexusNotify({...}) // in nexus codebase

The onDiagnostic callback would receive a Diagnostic type.

This would be structured data about something. In the abstract:

interface Diagnostic {
  code: string
  level: 'error' | 'warn' | 'info'
  message: string
  context: PlainObject
}

Concretely using this case as an example https://github.com/graphql-nexus/schema/blob/481e2c77fea61213b011d21659359fa425cefc55/src/utils.ts#L389.

interface MissingResolveTypeImplementation {
  code: 'NEXUS_ERR_ABSTRACT_TYPES_MISSING_RESOLVE_TYPE'
  level: 'error'
  message: string
  context: {
    typeName: string
    location: {
      module: string // path
      line: number
    }
  }
}

The code would have the following benefits:

  • turn parameter to onDiagnostic into a discriminant union for great autocompletion/data discovery/type safety
  • Improve communication in the community by having more precision when talking about things
  • pair with https://nxs.li shortlinks to link to an index of diagnostics with additional detail and guides/guide links.

When user supplies own onDiagnostic it would remove Nexus' default one.

// silence diagnostics
onDiagnostic(diagnostic) {}

User could throw if they wish to enforce checks in CI

onDiagnostic(diagnostic) {
  if (isCI() && diagnostic.level === 'error') throw new Error(diagnostic.message)
}

User would have access to a second parameter, the original diagnostic printer, thus allowing them to tap into diagnostics without giving the defaults up.

// throw on error diagnostics
onDiagnostic(diagnostic, printer) {
  telemetry.send(diagnostic)
  printer.print(diagnostic)
}

There is a lot of room to play with the printer api. For example maybe it has the built in concept of throwing:

onDiagnostic(diagnostic, printer) {
  printer.print(diagnostic, {
    throwWarnings: isCI(),
    throwErrors: isCI(),
  })
}

The name diagnostic is taken from TypeScript terminology.

@jasonkuhrt jasonkuhrt added type/feat Add a new capability or enhance an existing one note/breaking-change This issue existed already labels Nov 20, 2020
@jasonkuhrt
Copy link
Contributor Author

Related #616

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
note/breaking-change This issue existed already type/feat Add a new capability or enhance an existing one
Projects
None yet
Development

No branches or pull requests

1 participant