-
Notifications
You must be signed in to change notification settings - Fork 5
Coder plugin: refactor codebase to use API factories #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Brain dump from previous message. You don't need to read this unless I suddenly get hit with a bus, in which case, hopefully this is detailed enough ContextThe more I've work with Backstage, the more I've realized that it was a mistake not to use their API factories. When I was first putting the plugin together, I was focused on doing things the "traditional React way", and not the "Backstage way". All the API logic is also thrown into one file, and I think it's getting to the size where it should be split up, now that we've seen a few different use cases play out. As of this writing, the plugin does not use any custom API factories, which is the main system that Backstage leverages. API Factories are somewhat similar to React Context, in that they're also a dependency injection mechanism, but Backstage is built primarily with factories and not Context. And because the Coder plugin is built mainly with Context, that limits their ability to talk to each other, and let the user swap pieces out when wiring up the plugin. Parts of an API factoryWhen wired up correctly, a single API factory consists of three parts. The first two have some pitfalls when trying to show UI state, which I'll highlight in the 'challenges' section below.
Benefits to doing a refactorRefactoring the codebase to (1) split it up, and (2) use these factories should give us the following benefits:
ChallengesChallenges in UI codeAs mentioned above, the vast majority of API classes are "function buckets". That is, they can be stateful, but:
Challenges for testingBackstage's testing documentation still has a lot of TODO sections, so there isn't any official guidance on using custom API factories, but I know for a fact that you can inject them during testing. It's part of the reason why API factories exist at all. I don't expect this part to take too much time, but I will need to look through the source code to see how the classes from the core Backstage plugins are wiring things up Proposed solution for initial refactorI already have a separate branch with a lot of the changes already made, but my solution consists of a few parts:
|
Update on this: right now, I have a branch that tackles this issue and #114 at once. But with the Coder SDK issues, now both are blocked. I could split these up, but without the Coder SDK, I don't know if API factories have enough immediate impact to justify undoing some of the work, and then redoing it once the SDK is available. Happy to change course if there's disagreement, but I think this can be parked for a little bit. It won't block other Backstage issues like the Coder buttons |
Part of umbrella issue #16
Bleeds into #114 a little bit.
These changes are going to take a while, but they are close to done. Adding API factories should require some slightly complicated upfront work, but should make it easier to add new features over time, while making our plugin align more with modern Backstage community practices.
Requirements
Should have
The text was updated successfully, but these errors were encountered: