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

HIDDevice class - problem using "this" as named parameter in method declarations #77

Open
davisford opened this issue Sep 7, 2021 · 2 comments

Comments

@davisford
Copy link

This is a TypeScript project bootstrapped with create-react-app, here are my imports:

├── @material-ui/[email protected]
├── @material-ui/[email protected]
├── @testing-library/[email protected]
├── @testing-library/[email protected]
├── @testing-library/[email protected]
├── @types/[email protected]
├── @types/[email protected]
├── @types/[email protected]
├── @types/[email protected]
├── @types/[email protected]
├── @types/[email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

Here is my tsconfig.json

{
  "compilerOptions": {
    "target": "es5",
    "lib": [
      "dom",
      "dom.iterable",
      "esnext"
    ],
    "allowJs": true,
    "skipLibCheck": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "noFallthroughCasesInSwitch": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "noEmit": true,
    "jsx": "react-jsx"
  },
  "include": [
    "src"
  ],
  "files": [
    "src/typings/index.d.ts"
  ]
}

The issue appears to be the declaration of the oninputreport function on the HIDDevice class as such:

/*~ https://wicg.github.io/webhid/#hiddevice-interface */
declare class HIDDevice extends EventTarget {
    oninputreport: ((this: this, ev: HIDInputReportEvent) => any) | null;
    // etc
}

Using this here seems to constrain the function to force it to be implemented either on HIDDevice or else if implemented on another class it forces it to use this as the parameter name, which causes other problems...here's an example of what I mean:

Screen Shot 2021-09-07 at 10 03 45 AM

Note above the TS compiler is complaining that my implementation of that function is not assignable b/c the parameter name is not matching. I attempted to underscore it, but it still does not like it:

Screen Shot 2021-09-07 at 10 04 03 AM

So, I tried to override it using a typings file, but that still isn't working. The only hack workaround at this point is to cast it to unknown and then cast it back which is realistically a very poor hack.

Screen Shot 2021-09-07 at 10 09 24 AM

Of course, if I name the parameter this in my function implementation, well this causes me other issues:

Screen Shot 2021-09-07 at 10 18 03 AM

I can no longer use this as intended on my own class.

The fix here seems fairly simple. In the spec / implementation of the typings, you could re-do the declaration like so:

/*~ https://wicg.github.io/webhid/#hiddevice-interface */
declare class HIDDevice extends EventTarget {
    oninputreport: ((dev: HIDDevice, ev: HIDInputReportEvent) => any) | null;
    // etc
}

I see several other areas of the typings spec where this is used quite a bit. I would advise against this if possible.

@davisford
Copy link
Author

Just out of curiosity, is there a specific reason to have the first parameter to the oninputreport method at all? The HIDInputReportEvent object itself has a reference to the HIDDevice, so it seems redundant / un-necessary.

@cmawhorter
Copy link

this has special meaning in typescript.

additionally, oninputreport is a property and not a method. you'd probably want to do MySubclass: ctor { this.addEventListener('inputreport', this._onInputReport.bind(this)) }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants