-
-
Notifications
You must be signed in to change notification settings - Fork 8.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
fix(runtime-dom): enable set form attr to null on form-elements (#2840) #2849
Conversation
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.
Looking good, thanks!
Just a typo needing to be fixed, see comment.
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.
Typo need to be fixed, see comment.
Edit: commited the fix myself.
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.
This should be good now!
Co-authored-by: Eduardo San Martin Morote <posva@users.noreply.github.com>
I thought about this a bit more, and I think this solution is good for this individual issue, but there are more readonly properties out there that could fail like this. Example: #2766 I think the core reason is that the last line of Instead of adding new exceptions for individual properties like we do here, we could do something like this: return key in el && !!Object.getOwnPropertyDescriptor(el.__proto__, key)?.set But I'm not 100% sure if thats a) performance and b) safe for all elements. It works for not the Is there another (better?) way to check if a prop is readonly? |
We could still cache it for perf reasons but I wonder if there isn't an exception in a particular browser that makes the property readonly without setting the proper flag (you never know with JS!) I don't find having a list of exception that bad unless it begins to get too big. It also allows for specific checks and better handling of edge cases |
agreed. I sent a seperate PR for the textarea thing. This one here is again good to go. |
Removed tag check in 180310c The form tag list adds to the baseline size and isn't really worth it. |
Close #2840