Refactoring Storage and Providers

[stramel]

Hey @six7! 👋🏼

I was looking into contributing another provider to the repo. It currently looks like much of the storage/provider logic is distributed heavily raising the bar for contribution of new providers.

I was curious if you would be open to accepting a contribution that would refactor this into a more centralized area for each provider. The hope would be that each storage/provider would only need 3-4 files colocated to be created and very minor changes in some unions in other spots.

After doing this, I would like to contribute another storage/provider option.

[six7]

Hey @stramel - thanks for offering this! Can you outline how you'd like this to be? I'm all for optimizing the developer experience on that one. I'll also loop in @LiamMartens in this since he refactored it recently.

[stramel]

@six7 @LiamMartens Hi, sorry for the delay in response. I wanted to really dig into the repo to provide a good outline of what I was thinking.

Here is a summary of what I'm proposing:

Ideal Outcomes

• Colocation
• Minimal file modifications
• Simplify required knowledge to implement

Proposal:

Currently, the integrations are spread throughout the repo separated by the core logic and the react app (from what I can tell). This makes it hard to track what all needs updated and what all is necessary to be created when looking to add a new integration. I'm proposing colocating the integrations so all files are easy to find and maintain. Something like

* src/integrations/
    * X/
        * index.ts
        * storage.ts
        * hook.ts
        * Form.tsx
        * SyncMessage.tsx
    * index.ts
    * tokenStorage.ts
    * types.ts
    * constants.ts
    * helpers.ts

This would allow each integration to have its own folder and to only modify files within the src/integrations folder. All other places that need the some kind of information about integrations, would be accessed via helpers or the integration hook.

The hooks would encapsulate any UI logic/events, and follow a common API contract. Everything else should be maintained within the storage and other files.

It looks like the main thing that other areas are looking up is the proper name of the integration for display purposes. There are a few spots that are checking for "readOnly providers", "remote providers", "git providers", or "hosted providers". These could likely be pulled into a helper function within the integrations folder.

Moving Forward:

I think the best approach would be to do this in small steps:

• align on terminology (store/tokenStore/storage/provider/integration)
• moving to a colocated setup
• abstracting the distributed logic into helper functions within the integrations folder
• abstracting UI chunks into separate integration specific files

I have also identified a few other improvements that we could tackle after the initial move.

• Refactor the base class progressively migrate to the new base class while maintaining backwards compatibility
• Move multiFile logic into a reusable mixin
• Swap to Error classes for error handling
• Abstracting some repeated logic between integrations

Happy to hear feedback and discuss further. I have joined the slack channel if you would like to discuss via a huddle or in a channel for quicker iteration.

[six7]

I think that makes a lot of sense and would help other contributors a lot. I'll create a channel in Slack for this tomorrow so @LiamMartens can add his thoughts on this as well, I'm sure there's things that he had on his mind that can be further improved