Page MenuHomeElementl

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

Authored by dish on Nov 13 2020, 6:10 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 30, 11:44 AM
Unknown Object (File)
Sun, Nov 13, 5:55 AM
Unknown Object (File)
Sat, Nov 12, 11:05 AM
Unknown Object (File)
Sat, Nov 12, 10:00 AM
Unknown Object (File)
Nov 3 2022, 12:54 AM
Unknown Object (File)
Oct 27 2022, 8:50 AM
Unknown Object (File)
Oct 16 2022, 11:26 AM
Unknown Object (File)
Oct 4 2022, 2:05 PM
Subscribers
None

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
Branch
dish-id-fields (branched from master)
Lint
Lint Passed
Unit
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 (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