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

refactor to use Proxy (backwards compatible) #150

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

Conversation

BryanCrotaz
Copy link

@BryanCrotaz BryanCrotaz commented Dec 16, 2021

Refactored to use Proxy at each node in the tree. This means that get and set are not needed.

let cs = changeset(myModel);
cs.people[3].name.first = "Bryan";
cs.people[3].age++;
cs.isDirty; // true
cs.save();

Fabulous for being able to have nested components that make edits and are only aware of their scope.

In ember-changeset we won't need the changeset-get and changeset-set helpers any more, making changesets work with vanilla JS.

Refactoring again to be able to store errors deep in the tree so that nested components can see the errors relevant to them.

object-proxy-handler stores scalar changes and object proxies in Maps. For Ember I'll add a factory in the options so you can pass in a TrackedMap. Similarly TrackedArray for array-proxy-handler.

I'll need to add the hooks (and work out what they are) so that Ember Data relationships can be proxied correctly.

@BryanCrotaz
Copy link
Author

BryanCrotaz commented Dec 16, 2021

The interesting new functionality is shown in access.test.ts

The get and set methods could be deprecated in favour of plain js access.

@BryanCrotaz
Copy link
Author

refactoring to separate out the changeset class from the proxy class so that there's nowhere near as much api potentially stomping on keys from the original object

@BryanCrotaz
Copy link
Author

I've tweaked the code in some tests, but I don't think I've changed the intent.

There are tests that say they're for changes that check the errors, and vice versa in the next test along. There are tests that rely on the exact order of errors - I've changed in my PR to .contains rather than .isEqual(array of errors). To make the bulk pass quickly I've sorted the errors and changes alphabetically, but it would be better to remove this and fix the tests to not rely on ordering.

execute(): this;
isValidating: (key: string | void) => boolean;
prepare(preparedChangedFn: PrepareChangesFn): this;
save: (options?: object) => Promise<this | any>;
Copy link
Contributor

Choose a reason for hiding this comment

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

related to the v2 proposal and empowering users to use immutability when they see fit, I think we should maybe deprecated save, and move to a applyChangesTo(passedObject) method instead. That way folks could opt to never mutate their originating data, and also support various optimistic and pessmistic update flows.

let sourceData = {};

let changeset = createChangeset(sourceData);

changeset.foo = 1;

// whenever you want the sourceData (that maybe happens to be rendered elsewhere) to update:
changeset.applyChangesTo(sourceData); // this is the equiv of .save today, (except we don't call any underlying save method on the sourceData... cause that's a bonkers behavior anyway

// or if you wanted a different object to represent what would be the "next set of changes", like:
let previewChanges = {};
changeset.applyChangesTo(previewChanges);

// and then if you wanted both the original data and a preview of the thanges, you could:

let snapshot = changeset.snapshot();

@BryanCrotaz BryanCrotaz changed the title [WIP] refactor to use Proxy refactor to use Proxy (backwards compatible) Dec 21, 2021
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