Page MenuHomeElementl

[dagit] Use lint to enforce query inclusion of `id` field
ClosedPublic

Authored by dish on Nov 13 2020, 6:10 PM.

Details

Summary

In an effort to improve our Apollo caching, use id fields more consistently in GraphQL.

  • Add a few new id fields where we currently have a value that can be used, plus a couple others that seemed reasonable (please push back if I'm wrong)
  • Use the GraphQL eslint plugin to enforce that id is used whenever it is available on a type
  • Repair all lint errors by adding id as needed
  • Remove a handful manual ids from AppCache

Ideally we would be able to use this anywhere that Apollo currently shows a cache warning.

Test Plan

Lint, jest, ts. Load Dagit, navigate around and verify that things look fine.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dish requested review of this revision.Nov 13 2020, 6:25 PM

Oh this is cool - I didn't realize that there was a linter plugin for this.

I know Apollo will look for "name" or "key" as well and automatically use that, but I think that's not sufficient for pipelines. I wonder if it's preferable to put pipeline IDs in the graphql schema or to define custom ID generators on the apollo side over here (https://www.apollographql.com/docs/react/caching/cache-configuration/#generating-unique-identifiers) to prevent collision / weird autogen'd keys?

really missing that relay build step right now

lint errors look legit, but this looks reasonable.

This revision is now accepted and ready to land.Nov 17 2020, 4:06 PM

Lint suppressions on the worker.ts file