Describe the bug
Not a bug per se but an implementation detail that we called out in review. The code in question was already in prod, so we did not deem it necessary to hold up that PR. That said, I have not been back to investigate/fix this and wanted to make sure it did not go unresolved indefinitely.
To Reproduce
Steps to reproduce the behavior:
This is a code-level issue that isn't user facing, so is low priority.
The chief concern here is that, while in the original implementation we may have a single dep or two that we want to skip re-firing the event for, in future changes we could add additional dependencies to the hook that will also be skipped. This can cause erratic and difficult-to-debug behavior that we should avoid if possible.
Expected behavior
We'd remove this "skip-linting" comment and use all dependencies, or re-engineer the implementation a way that we can avoid whatever problems the original skip was intended to circumvent.
Describe the bug Not a bug per se but an implementation detail that we called out in review. The code in question was already in prod, so we did not deem it necessary to hold up that PR. That said, I have not been back to investigate/fix this and wanted to make sure it did not go unresolved indefinitely.
To Reproduce Steps to reproduce the behavior:
The chief concern here is that, while in the original implementation we may have a single dep or two that we want to skip re-firing the event for, in future changes we could add additional dependencies to the hook that will also be skipped. This can cause erratic and difficult-to-debug behavior that we should avoid if possible.
Expected behavior We'd remove this "skip-linting" comment and use all dependencies, or re-engineer the implementation a way that we can avoid whatever problems the original skip was intended to circumvent.
Debug logs N/A