WIP: Refactor to remove global state

[devknoll]

Opening PR just to track progress and interest, and promote feedback and discussion. The plan is to factor out Relay's global state.

...ignore the failing tests 😄

[josephsavona]

@devknoll - great to see you're thinking about this! Moving away from singletons will have to be done incrementally to keep everything working in the interim, but we're happy to provide support and help guide you through these PRs.

I can share an internal doc that I wrote detailing the high-level steps for removing singletons and global state, will follow-up tomorrow.

[devknoll]

👍 That'd be super. My current plan is something along the lines of:

PR 1

• Convert singletons to classes with members
• Change internal Relay call sites to use a specified instance (w/ fallback)

PR 2

• Introduce RelayContext component
• Change singleton accessors to issue a deprecation warning

PR 3

• Remove singleton accessors

This should hopefully make the process relatively pain free allowing the changes to flow upstream...

[josephsavona]

export the class

[josephsavona]

This is a great start. My overall feedback is that any singleton module that is converted to a class should export the class, and all call sites of that module should be updated to either instantiate an instance or be passed one from somewhere. To help understand the implications of each change and make this reviewable, would you mind sending one PR per module?

[devknoll]

Ahhh, sorry, wasn't ready yet 😉 I avoided changing exports to avoid an explosion of changes, and to ensure all of the existing tests would keep passing 😄

[josephsavona]

Haha ok :-)

Feel free to keep iterating here while you're exploring, and I'll add comments and feedback. When you're ready let's split into separate PRs.

[devknoll]

Can you clarify that you mean you want PRs at e.g. the RelayMutationTransaction.js level?

[josephsavona]

They don't necessarily have to be exactly one PR per module, but try to think about ways to split it up into incremental changes. Note that RelayStoreData is where we are aggregating Relay state, so you can store instances of the new classes on RelayStoreData during the transition. For example if you convert RelayPendingQueryTracker to a class and need a place to store the instance (bc you haven't converted QueryRunner yet), you can store the RelayPendingQueryTracker instance in RelayStoreData.

[josephsavona]

@devknoll In addition to the previous issue to go with this PR, also take a look at #559 which describes our thoughts on drawing a clear boundary between Relay Core and the Relay/React integration. The proposed RelayContext there is a merge of the operations supported by the current Relay.Store and RelayStoreData.

[devknoll]

Closed this to open smaller & more focused PRs.