test(visitor-keys): add test to guard redundant visitor keys#10650
test(visitor-keys): add test to guard redundant visitor keys#10650antfu wants to merge 2 commits intotypescript-eslint:mainfrom
Conversation
|
Thanks for the PR, @antfu! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
View your CI Pipeline Execution ↗ for commit a3ff943.
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit a3ff943.
☁️ Nx Cloud last updated this comment at |
bradzacher
left a comment
bradzacher
left a comment
There was a problem hiding this comment.
the keys aren't redundant!
unionWith will preserve our ordering for the keys.
this is VERY important because the ordering defines the order in which the properties are visited. An incorrect order can cause incorrect rule behaviour or incorrect scope analysis because the code is not visited in source order.
// say this is what eslint-visitor-keys exports
const KEYS = { a: ['a'] };
unionWith({}) == KEYS
unionWith({ a: [] }) == KEYS
unionWith({ a: ['b'] }) == { a: ['b', 'a'] }
unionWith({ a: ['b', 'a', 'c'] }) == { a: ['b', 'a', 'c'] }By removing the keys you will cause unionWith to append missing keys to the end of the array -- causing a broken visit order!
This is why the tests are failing.
|
I see. Thanks for pointing out! |
2 participants
PR Checklist
Overview
From #10649 I found there are quite some redundant visitor keys that mostly might due to the updates in
eslint-vistior-keys. In order to locate them, I write a simple test to guard them - not sure if you consider them valid testing. With the tests, in this PR, I also removed the keys that are considered redundant.Feel free to close this PR if it's not desired