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

Deprecate useDisposable #676

Merged
merged 16 commits into from
Jan 30, 2024
Merged

Deprecate useDisposable #676

merged 16 commits into from
Jan 30, 2024

Conversation

saskliutas
Copy link
Member

Changes

Deprecated useDisposable hook. It is not compatible with React 18 StrictMode and there is no way to make it work correctly without changing API. Updated useOptionalDisposable to work correctly. Made some changes to useDisposable hook to make it work in React 18 StrictMode as best as it can. Current implementation will leak some disposable resources in StrictMode but should work correctly when StrictMode is disabled.

Deprecated useTreeEventsHandler because it was based on useDisposable hook. Added new useControlledTreeEventsHandler hooks to replace it.

Testing

Ran modified tests in Presentation repo that uses useDisposable hook https://github.com/iTwin/presentation/blob/master/packages/components/src/test/hooks/UseRulesetRegistration.test.ts

@saskliutas saskliutas requested a review from grigasp January 15, 2024 14:32
@saskliutas saskliutas requested review from a team as code owners January 15, 2024 14:32
@saskliutas saskliutas changed the title Deprecate use disposable Deprecate useDisposable Jan 15, 2024
Copy link
Contributor

@raplemie raplemie left a comment

Choose a reason for hiding this comment

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

Can you add a note in NextVersion.md

@saskliutas
Copy link
Member Author

Can you add a note in NextVersion.md

@raplemie Added note to NextVersion.md

@GerardasB GerardasB requested a review from raplemie January 30, 2024 11:06
Copy link
Collaborator

@GerardasB GerardasB left a comment

Choose a reason for hiding this comment

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

I'd appreciate it if you'd add a PR number to changes in NextVersion.md while resolving the merge conflicts (i.e. #700)

@GerardasB GerardasB dismissed raplemie’s stale review January 30, 2024 12:48

Suggestions applied.

@GerardasB GerardasB merged commit 6481aae into master Jan 30, 2024
19 checks passed
@GerardasB GerardasB deleted the deprecate_use_disposable branch January 30, 2024 12:49
@GerardasB GerardasB added the minor Changes in this PR requires creating a minor release label Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Changes in this PR requires creating a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants