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.

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

[Proposal] Unified Client interface #56

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@
"form-data": "https://github.com/spark/form-data/archive/v1.0.0-relative-path.tar.gz",
"stream-http": "https://github.com/spark/stream-http/archive/v2.2.1.tar.gz",
"superagent": "^2.2.0",
"superagent-prefix": "0.0.2"
"superagent-prefix": "0.0.2",
"verror": "^1.9.0"
},
"browser": {
"http": "stream-http",
Expand Down
2 changes: 1 addition & 1 deletion src/Client.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Particle from './Particle';
import Library from './Library';
import Library from './models/Library';

export default class Client {
constructor({ auth, api = new Particle() }) {
Expand Down
49 changes: 49 additions & 0 deletions src/collections/Base.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import NotImplementedError from '../errors/NotImplementedError';

/**
* @interface
*/
export default class Base {
/**
* @param {Object} [filter] Predicate used when fetching
* @return {Promise}
*/
fetch(filter={}) {
throw new NotImplementedError();
}

/**
* @property {boolean} hasNextPage Indicates if collection has a next page
*/
get hasNextPage() {
throw new NotImplementedError();
}

/**
* @return {Promise}
*/
nextPage() {
throw new NotImplementedError();
}

/**
* @property {boolean} hasPrevPage Indicates if collection has a previous page
*/
get hasPrevPage() {
throw new NotImplementedError();
}

/**
* @return {Promise}
*/
prevPage() {
throw new NotImplementedError();
}

/**
* @property {number} page Indicates current page number
*/
get page() {
throw new NotImplementedError();
}
}
14 changes: 14 additions & 0 deletions src/errors/CustomError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import VError from 'VError';

class CustomError extends VError {
constructor() {
super(...arguments);
this.name = this.constructor.name;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work without babel transpiling? anything in the JS spec that says a constructor name equals the name of the subclass?

}

static matches(error) {
return error.name === this.name;
}
}

export default CustomError;
3 changes: 3 additions & 0 deletions src/errors/NotImplementedError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import CustomError from './CustomError';

export default class NotImplementedError extends CustomError {}
11 changes: 11 additions & 0 deletions src/models/Base.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export default class Base {
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps we call it BaseModel to be more descriptive?

constructor(client, data) {
// Make client non-enumerable so it doesn't show up in Object.keys, JSON.stringify, etc
Object.defineProperty(this, 'client', { value: client });
this._assignAttributes(data);
}

_assignAttributes(data) {
Object.assign(this, data);
}
}
12 changes: 12 additions & 0 deletions src/models/BuildTarget.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Base from './Base';

/**
* @property {string} version
* @property {number} platformId
* @property {boolean} featured
* @property {boolean} prerelease
* @property {string} vendor
*/
export default class BuildTarget extends Base {

}
File renamed without changes.
2 changes: 1 addition & 1 deletion test/Client.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {expect, sinon} from './test-setup';

import Client from '../src/Client';
import * as fixtures from './fixtures';
import Library from '../src/Library';
import Library from '../src/models/Library';

let api;
const token = 'tok';
Expand Down
9 changes: 9 additions & 0 deletions test/errors/NotImplementedError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import {expect} from '../test-setup';
import NotImplementedError from '../../src/errors/NotImplementedError';

describe('NotImplementedError', () => {
it('matches the error', () => {
let exception = new NotImplementedError();
expect(NotImplementedError.matches(exception)).to.be.true;
})
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Add tests for error chaining

2 changes: 1 addition & 1 deletion test/Library.spec.js → test/models/Library.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {expect} from './test-setup';
import Library from '../src/Library';
import Library from '../../src/models/Library';

let client = {};
describe('Library', () => {
Expand Down