-
Notifications
You must be signed in to change notification settings - Fork 1.3k
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
ESM Attribute events #6913
Comments
What would it do under the hood? |
I'd guess something similar to the current const watch = (target, prop) => {
const privateProp = `#{prop}`;
target[privateProp] = target[prop];
Object.defineProperty(target, prop, {
set(value) {
if (target[privateProp] !== value) {
target.fire(`changed:${prop}`, value);
target[privateProp] = value;
}
},
get() {
return this[privateProp];
}
});
} |
Feels kinda useful, even though easy enough and cleaner to do by the user. |
We could just assign this , have updated the implementation
Yep arguably we could just include this as a reference in the docs, as opposed to providing an implementation and supporting all the edge cases. |
Often multiple properties are watched, and sometimes all of them: this.on('attr', (prop, value) => {
//
}); So if the goal is to provide API similarity, then try to mimic it as much as possible, e.g. instead of The ugly part is to write: |
Yep the V8/etc deopt is a valid point. It may be better for users to handle this manually through class getters/setters |
ESM Scripts by design do not trigger
attr:{propName}
events. In order to help migration, it would be helpful to provide some alternative. Something like;This allows user to attach events and can be applied to any class member, whether an attribute or not.
Any thoughts/input appreciated. @mvaligursky @willeastcott @Maksims @slimbuck
The text was updated successfully, but these errors were encountered: