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

fix: Update usePermission types for TS 4.4.2 compatibility #2101

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

scvnathan
Copy link

@scvnathan scvnathan commented Sep 1, 2021

Description

fixes #2092, #2056

If you want to test this out locally be sure to change the Typescript version to 4.4.2 in package.json

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as before)

Checklist

  • Read the Contributing Guide
  • Perform a code self-review
  • Comment the code, particularly in hard-to-understand areas
  • Add documentation
  • Add hook's story at Storybook
  • Cover changes with tests
  • Ensure the test suite passes (yarn test)
  • Provide 100% tests coverage
  • Make sure code lints (yarn lint). Fix it with yarn lint:fix in case of failure.
  • Make sure types are fine (yarn lint:types).

Copy link
Contributor

@kachkaev kachkaev left a comment

Choose a reason for hiding this comment

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

Thanks @scvnathan! I wonder if we can change typescript version in package.jsondevDependencies to 4.4.2 in this PR too. This will make sure that linting fails if the typings are still incompatible with the latest version. I hope that the build won’t become incompatible with older typescript versions, but I’m not 100% sure 🤔

@streamich WDYT?

src/usePermission.ts Outdated Show resolved Hide resolved
src/usePermission.ts Outdated Show resolved Hide resolved
@scvnathan
Copy link
Author

I wonder if we can change typescript version in package.json → devDependencies to 4.4.2 in this PR too.

Last I checked, there were a lot of other compatibility issues related to upgrading TS. I think its probably best to get this out the door to fix builds and then work on broader compatibility fixes in a different PR

@kachkaev
Copy link
Contributor

kachkaev commented Sep 1, 2021

Thanks for your contribution @scvnathan! The diff makes sense 💯

Let’s wait for @streamich to take a quick look and merge, I hope we can try out a new release soon 🙂

@kachkaev kachkaev changed the title refactor: Update usePermission types for TS 4.4.2 compatibility fix: Update usePermission types for TS 4.4.2 compatibility Sep 1, 2021
@kachkaev kachkaev requested a review from streamich September 4, 2021 16:14
@fnick851
Copy link

fnick851 commented Oct 7, 2021

@kachkaev Sorry to bother - but do you know if there's manual fix for this that I can temporarily apply, while we are waiting for a merge and release?

UPDATE: I enabled skipLibCheck in TS config to bypass it.

@kachkaev
Copy link
Contributor

kachkaev commented Oct 9, 2021

@fnick851 I’ve found some temp workaround in our project, but I can’t bring it up because I don’t have access to that codebase ATM. Perhaps @obfusticatedcode / @jquintozamora could share some details (from a PR diff)?

@maksbotan
Copy link

@streamich is there any hope to merge this and push a new release?

@kamranayub
Copy link

Workaround if not using usePermissions:

#2074 (comment)

@EdenTurgeman
Copy link

Is there a reason not to merge this? since this is still an issue
image

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

Successfully merging this pull request may close these issues.

Interface 'IMidiPermissionDescriptor' incorrectly extends interface 'PermissionDescriptor'
6 participants