streamich / react-use

React Hooks β€” πŸ‘
http://streamich.github.io/react-use
The Unlicense
41.79k stars 3.15k forks source link

Interface 'IMidiPermissionDescriptor' incorrectly extends interface 'PermissionDescriptor' #2092

Open zhangshichuan opened 3 years ago

zhangshichuan commented 3 years ago

What is the current behavior? image image image image

Steps to reproduce it and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have extra dependencies other than react-use. Paste the link to your JSFiddle or CodeSandbox example below:

What is the expected behavior?

A little about versions:

kachkaev commented 3 years ago

We're also seeing this in react-use 17.3.1 alongside typescript 4.4.2. Linting passes because CI still uses typescript 4.1.1. The error comes from:

https://github.com/streamich/react-use/blob/2cd91281afc5f252d9387dd4761e6f326b574f2c/src/usePermission.ts#L11-L19

That points us to #2063. @gelove what would be the right solution? It’s strange that lib.dom.d.ts sets

type PermissionName = "gamepad" | "geolocation" | "notifications" | "persistent-storage" | "push" | "screen-wake-lock";

That excludes "midi" etc. Not sure how to solve the typings apart from:

interface IMidiPermissionDescriptor extends Omit<PermissionDescriptor, "name"> {
    name: 'midi';
    sysex?: boolean;
}
interface IDevicePermissionDescriptor extends Omit<PermissionDescriptor, "name"> {
    name: 'camera' | 'microphone' | 'speaker';
    deviceId?: string;
}

Would that be semantically correct?

scvnathan commented 3 years ago

I've tested this out with TS 4.4 and this works too: https://gist.github.com/scvnathan/e9ff3a784fda80b408f514ab6f3acc68

I'm guessing these permission types are experimental so the TS team removed them

kachkaev commented 3 years ago

@scvnathan sounds reasonable! Would you be willing to submit a PR? πŸ‘

We can place type PermissionDesc = after interface Experimental...Descriptor definitions and possibly keep the original name (IPermissionDescriptor) to minimise the diff. TBH I prefer unabbreviated names without I/T prefix like ExtendedPermissionDescriptor, but we can leave this for later πŸ™‚

scvnathan commented 3 years ago

Sure, I can submit a PR πŸ‘

JoeDuncko commented 3 years ago

Hi! @react-hookz/web, the new library by one of react-use's former maintainers (background here and here) has resolved this outstanding type issue. It might be worth taking a look.

For those interested, there's an official migration guide for migrating from react-use to @react-hookz/web.

leepowelldev commented 3 years ago

Is this library offically unmaintained now? This bug is blocking our build pipelines - we can put in workarounds for now but it's not ideal.

JoeDuncko commented 3 years ago

@leepowelldev Unfortunately this library is not "officially unmaintained", as no one has been able to establish contact with @streamich (the project owner). As far as I can tell, every couple months he appears, accepts a PR or two, then disappears again. Personally, I tried contacting him multiple times to see if I could help with the project backlog. No luck. So I'd say this project is "for all intents and purposes, unmaintained".

leepowelldev commented 3 years ago

@JoeDuncko Sure. Kind of annoying as I'd happily move to @react-hookz/web, but the reality is many of the hooks we use in this library are not yet implemented.

JoeDuncko commented 3 years ago

If you compile a list of react-use hooks you use that are not yet in @react-hookz/web and send it my way? It'd help us figure out what hooks we should be focusing on next. Thanks!

leepowelldev commented 3 years ago

@JoeDuncko - thanks for this, just gone through and seems the only one we're missing that would allow us to migrate is useWindowSize, if I have time over the next day or two I can pull together a PR for this unless you already have one in the works?

JoeDuncko commented 3 years ago

@JoeDuncko - thanks for this, just gone through and seems the only one we're missing that would allow us to migrate is useWindowSize, if I have time over the next day or two I can pull together a PR for this unless you already have one in the works?

Awesome! I don't think we have it actively in the works, so a PR would be appreciated!

Rey-Wang commented 3 years ago

@kachkaev Hello, Is there a plan to merge the PR and release a new version?

kachkaev commented 3 years ago

@Rey-Wang I believe that https://github.com/streamich/react-use/pull/2101 should fix this issue. I am able to merge the PR, but only @streamich can publish a new version of react-use on npm. Looks like we’ll need to wait for his help here.

jamesthomsondev commented 3 years ago

@leepowelldev Unfortunately this library is not "officially unmaintained", as no one has been able to establish contact with @streamich (the project owner). As far as I can tell, every couple months he appears, accepts a PR or two, then disappears again. Personally, I tried contacting him multiple times to see if I could help with the project backlog. No luck. So I'd say this project is "for all intents and purposes, unmaintained".

I just started using this lib and ran into this build problem (annoyingly I don't even use usePermission, but it still won't build because of it). Disappointing that such a great lib is being thrown away due to inactive maintenance. GitHub should introduce some sort "claim abandoned repo" feature (yes, I know you can fork but it's not really the same). Ah well, guess I'll have to switch to @react-hookz/web...

sidhuko commented 2 years ago

As a workaround to this you can change your imports to direct imports:

// @deprecated author doesn't maintain react-use actively.
// use @react-hookz/web instead
import useKey from 'react-use/lib/useKey';
import useDebounce from 'react-use/lib/useDebounce';
import useClickAway from 'react-use/lib/useClickAway';
import useTimeoutFn from 'react-use/lib/useTimeoutFn';
import useEffectOnce from 'react-use/lib/useEffectOnce';
import useWindowSize from 'react-use/lib/useWindowSize';
import usePreviousDistinct from 'react-use/lib/usePreviousDistinct';

export { useKey, useDebounce, useClickAway, useTimeoutFn, useEffectOnce, useWindowSize, usePreviousDistinct };

As long as you're not using usePermission you can stop the build issue. You can migrate to new library as they become available or start to write your own replacements.

Suzan-Dev commented 2 years ago

As a workaround to this you can change your imports to direct imports:

// @deprecated author doesn't maintain react-use actively.
// use @react-hookz/web instead
import useKey from 'react-use/lib/useKey';
import useDebounce from 'react-use/lib/useDebounce';
import useClickAway from 'react-use/lib/useClickAway';
import useTimeoutFn from 'react-use/lib/useTimeoutFn';
import useEffectOnce from 'react-use/lib/useEffectOnce';
import useWindowSize from 'react-use/lib/useWindowSize';
import usePreviousDistinct from 'react-use/lib/usePreviousDistinct';

export { useKey, useDebounce, useClickAway, useTimeoutFn, useEffectOnce, useWindowSize, usePreviousDistinct };

As long as you're not using usePermission you can stop the build issue. You can migrate to new library as they become available or start to write your own replacements.

Thanks!