Skip to content
This repository was archived by the owner on Dec 27, 2022. It is now read-only.

Server plugins#47

Open
saimonmoore wants to merge 7 commits intobluelinklabs:masterfrom
saimonmoore:server-plugins
Open

Server plugins#47
saimonmoore wants to merge 7 commits intobluelinklabs:masterfrom
saimonmoore:server-plugins

Conversation

@saimonmoore
Copy link
Copy Markdown
Contributor

What?

Proposal for extending ctzn servers via server plugins.

The server plugins (or extensions) are npm packages that can be enabled via configuration.

I created an example server plugin package here: https://github.com/saimonmoore/ctzn-server-plugin-example complete with actual examples of working code and passing tests.

See the readme of the example plugin for instructions on how to get it all setup.

The basic gist:

  • There are a few entry points where plugins can insert and extend key parts of ctzn: db classes, schemas, db views, api, app etc
  • Each package exports a public api of functions and each of these functions (save that of schemas) will have the setup method called on it.
  • Certain helpers (network, strings), db, schema helpers etc are also now exported by the ctzn package. Ideally, these would be extracted into a ctzn-lib package (I envision ctzn-lib, ctzn-db, ctzn-server packages in the future).

Note: I've not actually published the package yet. Basically because I obviously want your feedback and this PR and any future changes required would need to be merged so the example server plugin can actually depend on a version of ctzn with support for server-plugins.

And tests are passing :)

Let me know what you think. I just rebased with your latest push.

Next steps

If you are happy with the proposal and it eventually makes it way in, then I'll:

  • publish the ctzn-server-plugin-example on npm
  • Create another PR to add this package as a dependency to ctzn
  • Write up some documentation to make it easy for others to get started with their own plugins.

@pfrazee
Copy link
Copy Markdown
Collaborator

pfrazee commented Mar 29, 2021

Excellent work! Very clear and close to how I'd do this myself.

Meta planning: When the beta stabilizes -- which should be by end of April -- my goal is to shift 50% of my time to working with contributors. That's when I'll be better equipped to develop this PR with you. That said, part of the beta deliverables is the core SDK, which this is a part of, so I'll probably turn my attention to this sooner (mid April?).

There's a lot of organizational/high-level details that I'll want to develop with you. Setting up the git/npm orgs and repos, things like that. In the mean time, I'll offer some smaller feedback you can tackle without me. Sound good?

Overall this is solid and I appreciate how carefully you've matched the codebase's style. Two things stand out to me as potentially improvable:

  1. Passing the extensions into the database constructors was a reasonable choice, but threading state like that seems more error-prone than using global state. I initially tried to build this repo to avoid global state, but ultimately gave up and created a global for the config. Unless you have an argument against this approach, I suggest we do the same for active extensions. For now, you can create some simple version of this; I'll probably rearch the repo when we finalize this PR so that the global state is more consistent across each component (ie a "server" global) and then I'll unify your solution with that.
  2. The camel-cased extensions for the different database types would be a good idea -- except that there's no constraint on how many database types there will be. We could do some magic of "thing.com/cool-db" -> thingComCoolDb but that seems less useful than just exporting according to the actual string of "thing.com/cool-db". WDYT?

@saimonmoore
Copy link
Copy Markdown
Contributor Author

Yep makes sense 👍

I'll take some time to look through your points in the meantime and get back to you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants