Back to Relay

Agenda

meta/meeting-notes/2019-01-25-oss-sync.md

20.1.17.8 KB
Original Source

Agenda

  • Meet with community collaborators.
  • Follow-up internal discussion on items from previous meeting.
  • Push 2.0.0 release.

This was our first Relay "OSS Day", where the core team devoted most of the day to catching up on OSS including meeting with several outside contributors to discuss our roadmap and important issues for the community as well as triaging issues/PRs and cutting a release. Notes are high-level and include points from both the broader discussion and internal follow-up discussions.

Attendees

Core team:

  • Jan Kassens
  • Andrey Lunyov
  • Gerald Monaco
  • Joe Savona
  • Juan Tejada
  • Yuzhi Zheng

Community members:

  • Sibelius Seraphini (@sibelius)
  • Rob Richard (@robrichard)
  • Justin (Artsy)

2019 H1 Roadmap

First we reviewed the roadmap discussed in https://github.com/facebook/relay/issues/2554#issuecomment-447957728 with the attendees. The main note here is that the new React Suspense-compatible APIs will be moved back to OSS as soon as possible.

Community Issue Triage

Next we reviewed issues/PRs that are impacting OSS Relay users and discussed possible next steps.

Support more flexible object identity

  • Justin: Ideally applications would be able to use Relay even if they don't conform to Relay's object identity spec.
  • Background: Relay expects objects to use the Node interface and id field as a way to uniquely identify entities. There are two main cases we would also like to support:
    • Applications that do use globally unique object identifiers but on a different field (ie not id).
    • Applications that don't use globally unique identifiers at all (they may have type-specific primary keys instead).
    • A proposed solution to the first case is implemented in https://github.com/facebook/relay/pull/2249, which proposed inferring the name of the global "id" field from the schema's Node interface. E.g. if the schema's Node interface calls the id field __id, Relay would use the __id field as the identifier.
  • Discussion:
    • The core team's main hesitation with this PR (and issue generally) is that it can introduce runtime overhead for those applications that do conform to the spec. It's more efficient to use a fixed name for the identifier field than to do an extra lookup or call a function.
    • But that means Artsy has to main a fork of Relay that changes from looking up id => looking up __id. More generally, we would ideally like to support applications that don't conform to the identity spec.
    • Another use-case is that even in applications that mostly conform to the spec, there may be occasional types that do not (for example if some types have non-globally unique 'id' fields).
  • Some options:
    • Fork (via module shimming, custom build, etc) the identity resolution logic for Facebook and OSS. The OSS version would call user-provided function to resolve object identity (with a default that matches the current behavior). OSS users are often more tolerant of small performance tradeoffs in exchange for flexibility, so this could be reasonable.
    • Add a variant of LinkedField that either specifies the name of the id field or indicates that a function should be called at runtime. Types that conform to the Relay object identity spec would use the standard LinkedField with lower overhead, and only the types that need customization would incur the overhead. This is a more flexible option as it preserves performance of existing cases while also enabling more flexibility.
  • Next Steps:
    • @josephsavona to follow-up on the PR

Server-side rendering

  • Rob Richard: Implemented server-side rendering with Relay but there are a few issues.
  • Discussion:
    • Found it more flexible to avoid QueryRenderer; manually fetch data, set context, and render, then serialize data to the client to seed the environment, and then similarly manually set context and render.
    • Limitation is that it nested QueryRenderers aren't prefetched on the server, but in general fragment composition means nested renderers aren't typically necessary on initial load.
    • Relay uses inline requires which is slow on Node.js; as of writing this is now changed so that the OSS build disables inline requires.
    • Relay was using an unstable API to read React context that didn't work, that's fixed to use a different unstable API that works for now. Relay team will update again once there's a final API to use.
    • Built-in support for defer would address the nested query renderer case.
    • Rob is using Apollo server's experimental @defer with a custom network layer to patch incremental payloads.
    • There's also a related issue with QueryRenderer's shared requestCache.
    • Upcoming new APIs / changes will make it easier to use QueryRenderer to synchronously render from data in the cache, and to manually set context if necessary.
  • Next steps:
    • @jstejada to look at the requestCache bug.

Mutation overlap

  • Rob Richard: The results of different mutations can overlap in the store. The recommended workaround is to use include a value for caching-breaking purposes in the mutation variables, for example a clientMutationId argument set to an auto-incrementing value.
  • PR to address this in https://github.com/facebook/relay/pull/2349, which sets a new unique root id for each mutation.
  • Discussion:
    • The core team is supportive of this direction.
  • Next Steps:
    • @josephsavona to follow-up on the PR

Making Relay easier to use

  • Sibelius: Frequently hears that Relay feels harder to use than Apollo.
  • What's actually harder?
    • Both Apollo and Relay support similar patterns (ie, declare a single query with all fields inlined vs colocated fragments w components). But somehow people don't realize they can just use a query with Relay, and add colocated fragments over time as their app grows.
    • People often don't understand the value of colocation and data-masking; it isn't a problem when you're just getting started, but it matters as an app scales.
    • Docs and examples could help to clarify that fragments/colocation/masking are advanced concepts you don't have to use at first.
  • What else is hard?
    • The compiler requires setup: PR to make it configurable discussed later.
    • Better docs: planned on the roadmap.
    • Examples? Community help appreciated.
    • Can we use babel-macros for the compiler? This is how create-react-app supports Relay.
    • If you're a React dev not already using GraphQL, there is a lot to set up already (schema, server library, client library, etc).
  • Next Steps:
    • Relay team to think about how to make the value proposition (and intended audience) more clear on the website/docs.

Local State

  • Sibelius: What are we thinking re local state?
  • The core team is exploring this in collaboration with the React team and other folks. Nothing concrete yet, but we are thinking about how complex local state should work in the presence of React Suspense and Concurrent mode.

Relay Compiler Options

  • https://github.com/facebook/relay/issues/2518 proposes an idea for how to configure the compiler via a config file similar to Babel, Metro, other JS utilities.
  • Discussion: Relay team in support, cosmiconfig library looks good, detailed writeup from the community would be appreciated per comments on the issue.

Frozen JSON scalars

  • Rob: https://github.com/facebook/relay/pull/2193 is open, this is an issue for some OSS users who use complex scalars (eg the JSON scalar).
  • Discussion:
    • The fix is to stop modifying frozen objects in DEV in recycleNodesInto.
    • It's technically an observable behavior change between DEV and production modes, which we historically have tried to avoid.
    • But this only occurs for custom scalar data - not standard Relay data.
    • Decided to move forward to unblock the community because complex scalars are already opt-in. Object identity for complex scalars is not guaranteed to be stable if you use this complex scalars. PR landed.