-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
ES6 Proxy #776
ES6 Proxy #776
Changes from all commits
15018ee
458c22e
3106ad5
5263bd3
bc78de3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,6 +116,35 @@ export class IObservableFactories { | |
return res as any; | ||
} | ||
|
||
dynamic<T>(props: T, name?: string): T & IObservableObject { | ||
if (arguments.length > 2) | ||
incorrectlyUsedAsDecorator("object"); | ||
invariant(typeof Proxy === 'function', "dynamic objects are not supported in this environment"); | ||
const internalMap = new ObservableMap(props, referenceEnhancer, name); | ||
const result = new Proxy(internalMap, { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could also add a |
||
get(target, property:string, receiver){ | ||
if (property === '$mobx'){ | ||
return target[property]; | ||
} | ||
return target.get(property); | ||
}, | ||
set(target, property:string, value, receiver){ | ||
if (property === '$mobx'){ | ||
return false;} | ||
target.set(property, value); | ||
return true; | ||
}, | ||
ownKeys: function(target) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is used by |
||
return target.keys(); | ||
}, | ||
getOwnPropertyDescriptor: function(target, property:string) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this necessary? Why not just leave the default property descriptor? If it is necessary shouldn't it include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. before we added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIK the rest of the fields have default fallbacks that fits our use case. |
||
return {enumerable: target.has(property), configurable:true}; | ||
} | ||
}); | ||
|
||
return result as any; | ||
} | ||
|
||
|
||
/** | ||
* Decorator that creates an observable that only observes the references, but doesn't try to turn the assigned value into an observable.ts. | ||
|
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.
Maybe check of the props object is already wrapped by a dynamic observable Proxy. Accidental double wrapping could cause some nasty bugs.
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.
I didn't think about that. thanks!