-
Notifications
You must be signed in to change notification settings - Fork 82
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
Object.defineProperty should trigger reactions #44
Comments
I am not sure about that. There are a few Did you get into a use case where this behavior surprised you or did you make this suggestion based on the source code? |
I just skimmed over your blog post and read the section you mentioned (A
Bit of Intuition).
Here's my use case: I'm using observer-util for a dynamic & reactive table
of data and expressions, similar to a spreadsheet. Data is stored as a
property assignment with expressions stored by defining a getter.
const spreadsheet = observable ({})
// both would trigger reactions properly
spreadsheet.A1 = 1000
spreadsheet.B1 = 2000
// redefinition fails to trigger a reaction on B1
Object.defineProperty (spreadsheet, `B1`, { get: () => spreadsheet.A1 + 500
})
For my use case, dynamic getters have a huge advantage that APIs using the
spreadsheet object don't have to care whether they're accessing a data
"cell" or an expression "cell".
I like what you're doing with this project by extending plain and intuitive
JavaScript with reactivity. While I may be missing a good reason not to
make the Object.defineProperty operation reactive, I do think allowing
dynamic getters would make observer-util even better.
…On Fri, Mar 6, 2020, 02:32 Miklos Bertalan ***@***.***> wrote:
I am not sure about that. There are a few get and set like operations
that are not covered by design:
https://itnext.io/the-ideas-behind-react-easy-state-901d70e4d03e.
Object.defineProperty is one of them. I think it is too meta level, we
can revise this though.
Did you get into a use case where this behavior surprised you or did you
make this suggestion based on the source code?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#44?email_source=notifications&email_token=ABXYF3QQN7NLHMXU5O4XLSDRGC7LPA5CNFSM4IY5CEQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOAWJZY#issuecomment-595682535>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABXYF3R5WGUUJ75JNJJ5NE3RGC7LPANCNFSM4IY5CEQA>
.
|
Additionally, I'd point out that your plainObject = raw (observableProxy)
accessor API can cover for circumstances where it's desirable to use
Object.defineProperty (or any other operation) in a non-reactive manner.
Fwiw, let me know what you think.
…On Fri, Mar 6, 2020, 09:39 Robbie W ***@***.***> wrote:
I just skimmed over your blog post and read the section you mentioned (A
Bit of Intuition).
Here's my use case: I'm using observer-util for a dynamic & reactive table
of data and expressions, similar to a spreadsheet. Data is stored as a
property assignment with expressions stored by defining a getter.
const spreadsheet = observable ({})
// both would trigger reactions properly
spreadsheet.A1 = 1000
spreadsheet.B1 = 2000
// redefinition fails to trigger a reaction on B1
Object.defineProperty (spreadsheet, `B1`, { get: () => spreadsheet.A1 +
500 })
For my use case, dynamic getters have a huge advantage that APIs using the
spreadsheet object don't have to care whether they're accessing a data
"cell" or an expression "cell".
I like what you're doing with this project by extending plain and
intuitive JavaScript with reactivity. While I may be missing a good
reason not to make the Object.defineProperty operation reactive, I do
think allowing dynamic getters would make observer-util even better.
On Fri, Mar 6, 2020, 02:32 Miklos Bertalan ***@***.***>
wrote:
> I am not sure about that. There are a few get and set like operations
> that are not covered by design:
> https://itnext.io/the-ideas-behind-react-easy-state-901d70e4d03e.
> Object.defineProperty is one of them. I think it is too meta level, we
> can revise this though.
>
> Did you get into a use case where this behavior surprised you or did you
> make this suggestion based on the source code?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#44?email_source=notifications&email_token=ABXYF3QQN7NLHMXU5O4XLSDRGC7LPA5CNFSM4IY5CEQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOAWJZY#issuecomment-595682535>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABXYF3R5WGUUJ75JNJJ5NE3RGC7LPANCNFSM4IY5CEQA>
> .
>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Setting a new
value
or a newget
function usingObject.defineProperty
should trigger reactions on the relevant key. This should support reacting to both modifying existing keys of and adding new keys to an object.This would require adding a new Proxy trap and writing tests for the new behavior.
I'm willing and able to help put together a pull request.
Any thoughts?
The text was updated successfully, but these errors were encountered: