-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
base: main
Are you sure you want to change the base?
Conversation
The interesting new functionality is shown in access.test.ts The |
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 |
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>; |
There was a problem hiding this comment.
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();
Refactored to use Proxy at each node in the tree. This means that
get
andset
are not needed.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 thechangeset-get
andchangeset-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 forarray-proxy-handler
.I'll need to add the hooks (and work out what they are) so that Ember Data relationships can be proxied correctly.