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

wip: fake roc v2 api #1

Merged
merged 51 commits into from
Mar 29, 2019
Merged

wip: fake roc v2 api #1

merged 51 commits into from
Mar 29, 2019

Conversation

stropitek
Copy link
Contributor

@targos this is really a draft right now

I'm trying to have something more OO than the previous implementation. I'm trying to have an abstract class that the fake and real apis will extend. I'm not sure if it would be better to use inheritance (extends) or polymorphism (implements). I went for inheritance but I think polymorphism might actually have more sense because the implementations are very different and don't share much with a base class.

The basic idea is to have a Roc class that handles basic requests like getUser, and methods that return more complex objects, for example getDocument which provides methods and handles state for a single document. We could have another method getQuery which handles queries to roc.

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

All of this is very abstract for now. Are you asking if you should continue in this direction?
It doesn't seem bad but it's difficult to review without knowing what part of this is WIP and what is "ready".
For example, I find weird that getUser is in the abstract class but not getDocument and create.

const document = await roc.getDocument('uuid1');
const doc = await document.fetch(); // Fetch latest revision if it's out of sync
expect(doc).toEqual(data.uuid1[0]);
// await document.forceFetch(); // Fetch latest revision even if revision is up to date
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer an option to fetch: document.fetch({ force: true })

@stropitek
Copy link
Contributor Author

I think I'm not going to put a lot of state in Document class. Except for the synchronization feature (polling the document to know if it's still up to date). I feel like having:

document.addAttachment(...);
document.removeAttachment(...);
document.save();

would be a duplication of state since we would also need to have this state in redux. I'm going more for:

document.update(newContent, newAttachments, deletedAttachments);

so that document class remains almost stateless.

src/fake/Roc.ts Outdated Show resolved Hide resolved
src/fake/Roc.ts Outdated Show resolved Hide resolved
src/fake/Roc.ts Outdated Show resolved Hide resolved
src/Roc.ts Outdated Show resolved Hide resolved
@targos targos merged commit 064adb2 into master Mar 29, 2019
@targos targos deleted the v2 branch March 29, 2019 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants