Page MenuHomeElementl

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

Authored by dish on Nov 13 2020, 6:10 PM.
Referenced Files
Unknown Object (File)
Fri, Jan 27, 12:04 AM
Unknown Object (File)
Dec 29 2022, 9:17 AM
Unknown Object (File)
Dec 29 2022, 8:26 AM
Unknown Object (File)
Nov 30 2022, 11:44 AM
Unknown Object (File)
Nov 13 2022, 5:55 AM
Unknown Object (File)
Nov 12 2022, 11:05 AM
Unknown Object (File)
Nov 12 2022, 10:00 AM
Unknown Object (File)
Nov 3 2022, 12:54 AM



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

R1 dagster
dish-id-fields (branched from master)
Lint Errors
No Test Coverage

Event Timeline

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 ( 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