Skip to content

Conversation

@gzdunek
Copy link
Contributor

@gzdunek gzdunek commented Oct 17, 2025

Closes #34192 (funny, it's a 4th search result for "noImplicitThis" in Google)

In #59643 (comment) I had a bug caused by this being implicitly set to any.
To avoid such issues in the future, I enabled noImplicitThis rule.

return debounced;
}

interface MapCacheType {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to get rid of memoize because it had many this issues.
It was only used in one place, which was trivial to refactor, and this old-style code was hard to maintain (we already had a few any and TS/ESLint ignores there).

Copy link
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!


// Subsequent calls return the same value.
c.update();
expect(c.value).toBe(firstReturnValue);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't test the return value of update which runOnce explicitly caches, does it? 😏 We'd need to return the symbol from update instead of storing it in a property. But I guess you also want to test this, so maybe we do both? As in store the symbol on the property but also return it from update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, fixed!

@gzdunek gzdunek added this pull request to the merge queue Oct 20, 2025
Merged via the queue into master with commit 46dd5a0 Oct 20, 2025
42 checks passed
@gzdunek gzdunek deleted the gzdunek/no-implicit-this branch October 20, 2025 16:20
@backport-bot-workflows
Copy link
Contributor

@gzdunek See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Create PR
branch/v18 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable noImplicitThis setting of TypeScript

4 participants