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

TXN-1402: Fix infinite render loop in useInitializeProviders #82

Conversation

drichar
Copy link
Collaborator

@drichar drichar commented Jun 20, 2023

Description

If used correctly, we can get away with eslint-disable-next-line react-hooks/exhaustive-deps for the dependency array.

It mirrors the original implementation being abstracted by the hook. Provider config and node config shouldn't be changing once initialization is successful.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary

@drichar drichar merged commit 5cd72fd into release-2.0.0 Jun 20, 2023
1 check passed
@drichar drichar deleted the txn-1402-fix-infinite-render-loop-in-useinitializeproviders-hook branch June 20, 2023 05:25
drichar added a commit that referenced this pull request Jun 27, 2023
* TXN-1290: Fix failing Storybook build (#60)

* TXN-1289: Add unit tests (#61)

* TXN-1292: Update initializeProviders to be async (#64)

* fix(utils): initializeProviders race condition

The `initializeProviders` utility function needs to be async, so we can await each client's async
`init` method. Otherwise a race condition occurs and if the app mounts before initialization
completes, `reconnectProviders` is called too early.

BREAKING CHANGE: The exported `initializeProviders` function is now async and must be awaited and
called from inside a `useEffect` hook.

* docs(readme): update the README with new initialization examples

* TXN-1293: Update README (#65)

* TXN-1294: Improved initializeProviders API (#66)

* feat(utils): improved initializeProviders API

Users will be able to set client options and pass static instances of SDKs/libs via
initializeProviders.

closes #30

* feat: improved initializeProviders API (pt 2)

There's a lot happening here but it's all related so I can't break it up into bite-sized commits (I
tried). The high-level:

1. The mapped clients passed to the `WalletProvider` context provider are no longer wrapped in a Promise. It made functions accessing those clients async for no reason.

2. Provider configurations are strictly enforced with types, so a user can't pass incompatible client options or SDK instances to the wrong provider.

3. Added tests for initializing providers with a mix of IDs and `ProviderConfig` objects. The mock implementation of `client.init` returns a mock client that lets us verify each client was configured correctly. The mock clients are also used in the `useWallet` tests now.

4. Cleaned up the clients and their types a little and made minor changes to the base client, making it possible to test specifics of provider initialization.

* feat: useInitializeProviders hook

This simplifies the `WalletProvider` setup by handling all the `useState` and `useEffect` complexity inside a new custom hook.

* feat: update Daffi client to v2.0.0 API, closes TXN-1401

* chore: remove commented ProviderConfig type

* fix: pass clientOptions to mock clients where missing

* Updated README for v2.0.0 (#80)

* TXN-1402: Fix infinite render loop in useInitializeProviders (#82)

* build(rollup): fix unresolved dependencies error

* TXN-1399: WalletConnect 2.0 support (#83)

* chore: only import PROVIDER_ID from src/constants

* feat: WalletConnect 2.0 provider client

* fix(walletconnect2): catch "Modal closed" error in connect method

Closing the Web3Modal was throwing an uncaught error instead of rejecting the promise returned by
`connect`, leading to a runtime error. This refactors the connection logic using then-catch chaining
(using async/await would lead to `connect` returning  `Promise<Promise<Wallet>>`) so the error is
caught and the promise is rejected.

* feat(walletconnect2): nested transactions support

This updates the WalletConnect (2.0) provider client's `signTransactions` method to accept an array
of transaction groups, a change introduced in v1.3.1

* chore: remove WalletConnect v1 provider files and peer dependencies

* docs: update README

* chore: version bump to 2.0.0-alpha.0

* docs: next.js 13 "Module not found" error fix

* TXN-1404: Fix "No matching export" error for WalletConnect peer dependencies (#85)

To avoid requiring @walletconnect/utils and @walletconnect/types to be installed as peer
dependencies to support WalletConnect 2.0, the modules that were being used are removed and
replicated. The types package in particular wasn't a necessary dependency.

* chore: version bump to 2.0.0-alpha.1

* TXN-1406: Fix default providers if none specified (#86)

* fix: useInitializeProviders `providers` defaults to empty array

* chore: version bump to 2.0.0-alpha.2

* TXN-1407: Replace @walletconnect/sign-client with @web3modal/sign-html (#87)

* feat(walletconnect2): replace @walletconnect/sign-client with @web3modal/sign-html

The @web3modal/sign-html client handles all of the @walletconnect/modal logic as well as lots of the
complexity being handled manually in the previous WalletConnect provider client. And it's one peer
dependency to install vs two.

* fix(walletconnect2): make clientOptions type compatible with ProviderConfig

* test(walletconnect2): update mocks

* build(rollup): fix unresolved dependencies error

* chore(usewallet): remove unused PROVIDER_ID export

* docs: update WalletConnect instructions

* fix: PROVIDER_ID imports

* chore: version bump to 2.0.0-alpha.3

* TXN-1409: Debug mode (#89)

* chore: version bump to 2.0.0-alpha.4

* TXN-1412: Static imports are required (#90)

* feat(init): static imports are required

The cleanest, simplest solution to the build/compile errors encountered if any peer dependencies are
not installed is to require static imports. This is already a requirement for Remix apps, and it
will ensure that only the necessary wallet client/SDK libraries need to be installed by consuming
apps. This also removes the need for a "default configuration" and deciding which providers should
or shouldn't belong in it. Every wallet provider will need to be passed to the `providers` property
in useInitializeProviders, which is already how most apps do it.

BREAKING CHANGE: The `useInitializeProviders` must receive an object with the `providers` property
defined. Provider objects passed to `providers` must include a `clientStatic` property set to the
provider's client/SDK library, if it exists. For example, you cannot simply pass `PROVIDER_ID.PERA`,
it must be an object with both `id` and `clientStatic` set.

* test(init): fix failing tests

* fix(init): update NodeConfig type

The `nodePort` property can be a string or a number, and the optional `nodeHeaders` property is
added

* docs: update README

Updates code examples, provider configuration section, and migration guide to describe required
static imports. Various other changes as well.

* chore(example): update Storybook example to useInitializeProviders

* test(example): fix failing tests

* chore: rename ProvidersArray type

* chore(init): remove initializeProviders main export

The `useInitializeProviders` hook replaces `initializeProviders` as a helper function for
constructing the providers map that is passed to the context provider in consuming apps.

BREAKING CHANGE: Apps using `initializeProviders` will be forced to switch to
`useInitializeProviders`

* chore: version bump to 2.0.0-alpha.5

* docs: fix WCv2 modal themeMode setting in full example

* chore(deps): update dependency gh-pages to v5 (#76)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* TXN-1413: Remove unused dependencies, fix warnings (#91)

* chore(deps): update all non-major dependencies (#62)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update dependency @testing-library/react to v14 (#74)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update dependency babel-loader to v9 (#75)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(daffi): set shouldShowSignTxnToast to default

Version 2.x changed the default for this setting for Defly and Pera to match the libraries' default
(true). Daffi Connect had not been updated, now it matches the other two.

* chore(deps): upgrade zustand to ^4.3.8

This also updates how `create` is imported in the stores. The default import has been deprecated and
shows a warning if used in local dev.

* TXN-1414: Refactor InitParams type, tidy up provider class imports (#92)

* chore(deps): upgrade zustand to ^4.3.8

This also updates how `create` is imported in the stores. The default import has been deprecated and
shows a warning if used in local dev.

* refactor(types): single InitParams type that uses ProviderConfigMapping

This actually started as an attempt to resolve "circular dependencies" warnings that started
appearing after upgrading Rollup. It resulted in a refactored `InitParams` which extends
`CommonInitParams` with the `clientOptions` and `clientStatic` for each provider in
`ProviderConfigMapping`. It also resolves all but two of the warnings (will need to revisit) and
organizes the imports for each of the provider classes.

* chore(types): update `NodeConfig['nodeToken']` type to match algosdk

* TXN-1408: Switch to @walletconnect/modal-sign-html (#93)

The deprecated @web3modal/sign-html peer dependency is replaced by @walletconnect/modal-sign-html.
This is a like-for-like change, except for the renamed exports. The client's methods/API are the
same. The @walletconnect/modal-sign-html package uses a more recent version of @walletconnect/core
than the deprecated @web3modal/sign-html.

BREAKING CHANGE: @web3modal/sign-html will no longer work

* chore: version bump to 2.0.0-rc.1

* docs: update README

* chore: add banner image

* docs: add banner to README

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

1 participant