prefer-dom-node-dataset - Add support to optional chaining
#2158
Conversation
| > Output | ||
|
|
||
| `␊ | ||
| 1 | element.dataset.optionalChaining = "foo";␊ |
There was a problem hiding this comment.
We can't fix to this, it will break code.
There was a problem hiding this comment.
Hello Fisk, thanks!
How exactly can I help?
There was a problem hiding this comment.
We should either remove the fix for optional members or add a safe fix.
There was a problem hiding this comment.
I'm sorry @fisker, it took me a while to understand what was wrong, I think I got it now:
We should expect the autofix feature to keep the optional chaining, is that it?
Like
| 1 | element.dataset.optionalChaining = "foo";␊ | |
| 1 | element?.dataset.optionalChaining = "foo";␊ |
Did I get it right?
Thanks again for your review, cheers
There was a problem hiding this comment.
I don't think so, optional chaining can't be LHS.
There was a problem hiding this comment.
Let me think a little bit. If there is safe fix.
There was a problem hiding this comment.
element?.setAttribute('data-foo', 'bar')
Since foo?.dataset.foo = 'bar' is invalid, and we don't want auto fix to wrap it in a if, so we should just remove the autofix.
element?.getAttribute('data-foo')
Should be safe to fix to element?.dataset.foo
element?.removeAttribute('data-foo')
Should be safe to fix to delete element?.dataset.foo
element?.hasAttribute('data-foo')
Since Object.hasOwn(element?.dataset, 'foo') can cause runtime error, so I guess we should remove the autofix too.
There was a problem hiding this comment.
I'm adding hasOptionalChainElement utility in #2169, you can use it after we merge.
|
@gcunsolo Bump :) |
|
Bump |
|
Closing for lack of activity. |
999d3df failing tests on #2156
8170555 fix
...
3da90d7 snapshots update