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

Proxy Free API #439

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

camiboj
Copy link

@camiboj camiboj commented Oct 8, 2021

This is the PR created to start the iteration for including a new API without a proxy as discussed on #438 (cc @zhang123cnn @ept). *

Proxy Free API

Automerge uses JS Proxy extensively for its front-end API. However, to be able to support multiple JS runtime which does not support Proxy you can use the Proxy Free API.

This API does not modify the way automerge handles conflict or nested objects. It is only modifies the Object and Array APIs.

You can find documentation on this new API on proxy_free.md

test/test.js Outdated Show resolved Hide resolved
@zhang123cnn
Copy link

@ept as discussed, would be great to get you folks to review this PR :) Many thanks

@ept
Copy link
Member

ept commented Oct 12, 2021

Hi @camiboj, thank you very much for this contribution! As I mentioned on #438, we currently have a project underway to redesign the API between Automerge's frontend and backend, and this should allow the frontend to be substantially simplified. It might allow us to avoid using a proxy polyfill, and instead have a more straightforward proxy-free frontend that is a thin wrapper around the frontend-backend API.

If it's okay with you, I would like to keep this PR open and wait for the backend API redesign to land first. In that redesign we will keep in mind the desire for a proxy-free API. Once that redesign has landed, we will return to this PR and determine how best to structure the code to provide a choice between the current proxy-based API and the proxy-free API.

Sorry if that means you need to maintain a fork for a little while longer, but I think that approach will lead to the best results.

@zhang123cnn
Copy link

zhang123cnn commented Oct 13, 2021

Hey @ept thanks for your comment. I think that makes sense to us. We can wait until the redesign is finished to review this PR. (Do you have a ETA on the redesign timeline? Last time I remember you said a couple of weeks. It would be great to be able to track this externally)

At the same time, do you mind if we create a public fork in our facebook github organization? We need to host this forked code in github for our upcoming release. Having a public fork would unlock us right now. We will delete that fork once this PR is merged back to original repo though.

Please let us know what you think. Many thanks

CC @jtannady

@ept
Copy link
Member

ept commented Oct 13, 2021

No problem @zhang123cnn, feel free to make a fork!

@jtannady
Copy link

New fork created under the facebookincubator org. You can find it here.

https://github.com/facebookincubator/automerge

Thanks for all your support, @ept! 🎉

echarles pushed a commit to datalayer-externals/automerge-classic-arch that referenced this pull request Feb 16, 2023
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.

4 participants