Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable noImplicitThis setting of TypeScript #34192

Open
Tracked by #46670
ravicious opened this issue Nov 3, 2023 · 0 comments
Open
Tracked by #46670

Enable noImplicitThis setting of TypeScript #34192

ravicious opened this issue Nov 3, 2023 · 0 comments
Labels
developer-experience Addressing these issues will improve the experience of developers working on Teleport effort:low Tasks which can be completed within a few hours. ui value:low Tasks which completion brings comparatively low benefits.

Comments

@ravicious
Copy link
Member

https://www.typescriptlang.org/tsconfig#noImplicitThis

I feel we should enable this setting because writing code with implicit this makes it impossible for the TypeScript language server to find references for code which uses implicit this. See the example with this.clear() below.

With this setting enabled, I'm getting the following number of errors on master (cfc3480):

Errors  Files
    27  web/packages/shared/utils/highbar.ts:111
     2  web/packages/teleport/src/services/websession/websession.ts:156

Implicit this of an object

const session = {
logout(rememberLocation = false) {
api.delete(cfg.api.webSessionPath).finally(() => {
history.goToLogin(rememberLocation);
});
this.clear();
},
clear() {
this._stopTokenChecker();
localStorage.unsubscribe(receiveMessage);
localStorage.clear();
},

On the line with this.clear(), this is of type any. As such, if you want to find references to the clear function, this callsite won't be listed there. The solution to this is to replace this with session. In Connect, we did so for a long time since routing in uri.ts uses a similar pattern:

export const routing = {
parseClusterUri(uri: string) {
const leafMatch = routing.parseUri(uri, paths.leafCluster);
const rootMatch = routing.parseUri(uri, paths.rootCluster);
return leafMatch || rootMatch;
},

Implicit this inside a function

This happens mostly in highbar.ts and it's exactly what the docs of noImplicitThis describe. It can be solved by using an arrow function.

diff --git a/web/packages/shared/utils/highbar.ts b/web/packages/shared/utils/highbar.ts
index fda5f461a9..63bd24a5b6 100644
--- a/web/packages/shared/utils/highbar.ts
+++ b/web/packages/shared/utils/highbar.ts
@@ -103,19 +103,19 @@ export function isObject(checkVal: unknown): boolean {
  * Based on Underscore.js 1.8.3 <http://underscorejs.org/LICENSE>
  * Copyright Jeremy Ashkenas, DocumentCloud and Investigative Reporters & Editors
  */
-export function runOnce<T extends (...args) => any>(func: T) {
+export const runOnce = <T extends (...args) => any>(func: T) => {
   let n = 2;
   let result;
-  return function () {
+  return (...args) => {
     if (--n > 0) {
-      result = func.apply(this, arguments);
+      result = func.apply(this, ...args);
     }
     if (n <= 1) {
       func = undefined;
     }
     return result;
   };
-}
+};
 
 interface ThrottleSettings {
   leading?: boolean | undefined;
@ravicious ravicious added ui developer-experience Addressing these issues will improve the experience of developers working on Teleport labels Nov 3, 2023
@ravicious ravicious added effort:low Tasks which can be completed within a few hours. value:low Tasks which completion brings comparatively low benefits. labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Addressing these issues will improve the experience of developers working on Teleport effort:low Tasks which can be completed within a few hours. ui value:low Tasks which completion brings comparatively low benefits.
Projects
None yet
Development

No branches or pull requests

1 participant