-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
no-this-assign-in-render
: Only fail if the assignment is to a reactive property
#214
base: master
Are you sure you want to change the base?
no-this-assign-in-render
: Only fail if the assignment is to a reactive property
#214
Conversation
I don't work with a lot of this (including typescript) often, so suggestions and edits are welcome. |
} | ||
|
||
if (propertyMap.has(propertyName) || | ||
parent.property.type === 'Identifier' && parent.computed) { |
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.
There doesn't seem to be a great way to get the computed value (i.e. x
in this[x]
) to compare against the properties, so we fail on this as well.
a604e2d
to
d8f4313
Compare
@43081j Can you have a look at this when you get a chance? |
@43081j Any interest in moving this along? |
Sorry I've been quite busy traveling recently. I'll try get to this in the next few days 👍 |
the this does mean you're technically correct that we care most about reactive properties. but, really, I think we shouldn't be mutating @justinfagnani do you have thoughts on this? I think we should just fix the existing behaviour instead of constraining it to reactive properties. Justin can confirm but I feel like render just shouldn't mutate at all, really (of as far as I understand, you need this because we're incorrectly flagging things like |
Alternative/addition to #213
this
assignments only