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

Is repo import/export api needed? #64

Closed
jessegrosjean opened this issue Apr 28, 2024 · 2 comments · Fixed by #78
Closed

Is repo import/export api needed? #64

jessegrosjean opened this issue Apr 28, 2024 · 2 comments · Fixed by #78
Assignees
Labels
API feedback Feedback or recommendations for changes to public API

Comments

@jessegrosjean
Copy link
Contributor

jessegrosjean commented Apr 28, 2024

I'm reading through high level docs and seeing import/export API made me think there was some new concept I need to learn.

In my mind:

  • Instead of import it is clearer just to use repo.create(data: data)
  • Instead of using export it's clearer to just use docHandle.doc.save()
@jessegrosjean
Copy link
Contributor Author

Same question about repo.clone()? It also seems to be a new api/terminology that isn't super necessary. And the underlying implementation uses doc.fork() which is a detail that seems like it might be of interest to clients. In my mind it might make more sense to:

  • remove clone and client code can just using existing API repo.create(doc: docHandle.doc.fork())
  • or rename clone to repo.fork(id: id) so that API is reusing existing automerge terminology to describe more exactly what is happening.

Of course these are just outsider thoughts/suggestions without me knowing to much. Close and ignore as needed :)

@heckj
Copy link
Collaborator

heckj commented Apr 28, 2024

Thanks @jessegrosjean - super useful insight as usual!

I replicated the top-level functions (without change or really, any thought) from the Javascript automerge-repo. The import, export, and clone also seemed somewhat repetitive to me, being used to working with getting data in and out of an Automerge doc directly doc.save(), doc.fork(), etc.

In the JS side of the world, this is also more of a server component, and in particular there's some interesting wrangling of mixing together full syncs and "local changes" and persisting those separately (and later recombining them) that's embedded in the concept of the storage adapter - and I suspect that may have impacted the concept of the import and export - those are always as compacted documents, where the storage adapter works quite differently (but does support concurrent updates of multiple peers to a single filesystem location)

The notable factor of clone is that it makes a new document ID alongside the forked document, in essence doing exactly the same thing you're outlining in repo.create(doc: docHandle.doc.fork()) - so yeah, definitely repetitive with that concept.

@heckj heckj self-assigned this Apr 28, 2024
@heckj heckj added bug Something isn't working API feedback Feedback or recommendations for changes to public API and removed bug Something isn't working labels Apr 28, 2024
@heckj heckj closed this as completed in #78 May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API feedback Feedback or recommendations for changes to public API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants