Enable unicorn/no-array-method-this-argument in packages/tree #26181
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enables the unicorn/no-array-method-this-argument ESLint rule for the @fluidframework/tree package by removing the global override from ESLint configuration files and adding inline disables only where specifically needed for tests that verify thisArg binding behavior.
Changes:
- Removed the global
unicorn/no-array-method-this-argument: offoverride from both ESLint configuration files - Added inline ESLint disables for two test cases that explicitly test
thisArgbinding functionality
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/dds/tree/eslint.config.mts | Removed global override for unicorn/no-array-method-this-argument rule |
| packages/dds/tree/.eslintrc.cjs | Removed global override for unicorn/no-array-method-this-argument rule |
| packages/dds/tree/src/test/simple-tree/mapNode.spec.ts | Added inline ESLint disables for two test cases that specifically test thisArg binding behavior |
| } | ||
| // eslint-disable-next-line unicorn/no-array-for-each -- Testing thisArg binding behavior | ||
| // eslint-disable-next-line unicorn/no-array-for-each, unicorn/no-array-method-this-argument -- Testing thisArg binding behavior | ||
| node.forEach(callback, thisArg); |
There was a problem hiding this comment.
Can we not rewrite this as node.forEach(callback.bind(thisArg)); and remove the ignore? Or is this test specifically testing the thisArg binding behavrio?
There was a problem hiding this comment.
I believe it's testing when we pass in thisArg as the second argument, it gets correctly bound to "this" in the callback on line 61.
2 participants
Removes the unicorn/no-array-method-this-argument: off override from the @fluidframework/tree package and fixes the resulting errors. Adds inline disables for two test cases that specifically verify thisArg binding behavior. This is part of an ongoing effort to incrementally remove global ESLint rule overrides from eslint.config.mts and .eslintrc.cjs.
Since the 2 errors that result from this error removal are testing thisArg specificially, inlines are required. This proposed fix allows the rule to now be enabled for the rest of the package.