Skip to content
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.

Sign up for GitHub

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

Register Internal and Custom backend API's #1011

Merged
merged 3 commits into from
Jan 25, 2018
Merged

Register Internal and Custom backend API's #1011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3675933
allow backend register
talves Jan 15, 2018
32364a6
handle multiple backends, including internal
talves Jan 15, 2018
dd3a364
back to singular :smile:
talves Jan 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions src/backends/backend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ import { sanitizeSlug } from "Lib/urlHelper";
import TestRepoBackend from "./test-repo/implementation";
import GitHubBackend from "./github/implementation";
import GitGatewayBackend from "./git-gateway/implementation";
import { registerBackend, getBackend } from 'Lib/registry';

/**
* Register internal backends
*/
registerBackend('git-gateway', GitGatewayBackend);
registerBackend('github', GitHubBackend);
registerBackend('test-repo', TestRepoBackend);


class LocalStorageAuthStore {
storageKey = "netlify-cms-user";
Expand Down Expand Up @@ -339,15 +348,10 @@ export function resolveBackend(config) {

const authStore = new LocalStorageAuthStore();

switch (name) {
case "test-repo":
return new Backend(new TestRepoBackend(config), name, authStore);
case "github":
return new Backend(new GitHubBackend(config), name, authStore);
case "git-gateway":
return new Backend(new GitGatewayBackend(config), name, authStore);
default:
throw new Error(`Backend not found: ${ name }`);
if (!getBackend(name)) {
throw new Error(`Backend not found: ${ name }`);
} else {
return new Backend(getBackend(name).init(config), name, authStore);
}
}

Expand Down
23 changes: 23 additions & 0 deletions src/lib/registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { newEditorPlugin } from 'EditorWidgets/Markdown/MarkdownControl/plugins'
* Global Registry Object
*/
const registry = {
backends: { },
templates: {},
previewStyles: [],
widgets: {},
Expand All @@ -24,6 +25,8 @@ export default {
getEditorComponents,
registerWidgetValueSerializer,
getWidgetValueSerializer,
registerBackend,
getBackend,
};


Expand Down Expand Up @@ -87,3 +90,23 @@ export function registerWidgetValueSerializer(widgetName, serializer) {
export function getWidgetValueSerializer(widgetName) {
return registry.widgetValueSerializers[widgetName];
};

/**
* Backend API
*/
export function registerBackend(name, BackendClass) {
if (!name || !BackendClass) {
console.error("Backend parameters invalid. example: CMS.registerBackend('myBackend', BackendClass)");
} else if (registry.backends[name]) {
console.error(`Backend [${ name }] already registered. Please choose a different name.`);
} else {
registry.backends[name] = {
init: config => new BackendClass(config),
Copy link
Contributor

@tech4him1 tech4him1 Jan 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still wonder why we shouldn't just do

registry.backends[name] = BackendClass;

and then in the backend code

const UserBackend = getBackend(name);
return new Backend(new UserBackend(config), name, authStore);

Can you explain? Just cleaner?

Copy link
Collaborator Author

@talves talves Jan 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coder preference, I like the object reference is all. Kept it the way it was.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking initiative on this @talves, I love how simple of a first step this is!

Regarding this piece of code, we want project style to trump an individual developer's preference - no reason to make this function different than the others. Let's update this registry function to simply assign the registered entity as Caleb suggested.

After that, should be good to merge.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree.

Copy link
Contributor

@tech4him1 tech4him1 Jan 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine either way, it's pretty much just bikeshedding IMO. Do we have a reason that it should be one way or another?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be different if we weren't committing to this decision as a part of our public API, which we can't break until 2.0 once merged. We should be in agreement before proceeding - let's feature flag it for now and get some mileage on it before making it public.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like Tony said, this specific discussion is just about code style -- it doesn't impact the public API at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On not breaking the public API, though, I'm fine with putting it in a beta/flag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, this does not impact the public API, I stand corrected 🤦‍♂️

};
}
}

export function getBackend(name) {
return registry.backends[name];
}