Conversation
|
|
||
| ## [9.0.0] | ||
| ### Added | ||
| - **BREAKING**: Add required `getSelectedAddress` callback argument to constructor ([#1579](https://github.com/MetaMask/core/pull/1579)) |
There was a problem hiding this comment.
Nit: I can see how it makes sense to phrase this change entry as "Added required ___", but it feels more appropriate under the "Changed" category. Added is more often used for "new features" that can be taken advantage of, whereas this is a change in expectations for the caller. Likewise for the second entry as well.
There was a problem hiding this comment.
Great point, done.
|
|
||
| ### Changed | ||
| - **BREAKING**: Move all but first argument to options bag in `addTransaction` method ([#1576](https://github.com/MetaMask/core/pull/1576)) | ||
| - **BREAKING**: Update `fromBlock` and `limit` types in `RemoteTransactionSourceRequest` ([#1579](https://github.com/MetaMask/core/pull/1579)) |
There was a problem hiding this comment.
Nit: Reading this from the perspective of a mobile/extension maintainer, it's unclear how these have changed, or what actions I should take to accommodate these changes. If the "recommended action" for a breaking change isn't obvious, additional detail is greatly appreciated.
In this case it seems kinda moot, because it's a type that seems primarily meant for internal use. But from reading the changelog, it seemed like a change I'd need to handle at first.
Here's an attempt to explain that better, and also it corrects a mistake regarding the limit property (limit was unaffected, it was networkType that was removed)
| - **BREAKING**: Update `fromBlock` and `limit` types in `RemoteTransactionSourceRequest` ([#1579](https://github.com/MetaMask/core/pull/1579)) | |
| - **BREAKING**: Update properties of `RemoteTransactionSourceRequest` type ([#1579](https://github.com/MetaMask/core/pull/1579)) | |
| - The `fromBlock` property has changed from `string` to `number` | |
| - The `networkType` property has been removed | |
| - This type is intended mainly for internal use, so it's likely this change doesn't affect most projects |
There was a problem hiding this comment.
I'd skip the last added line here, more noise than signal imo: This type is intended mainly for internal use, so it's likely this change doesn't affect most projects.
Same as #1630 (comment)
Great expansion otherwise 👍
There was a problem hiding this comment.
I wasn't sure how verbose we should be in these changelogs, but this is indeed much clearer, done.
| - **BREAKING**: Update `fromBlock` and `limit` types in `RemoteTransactionSourceRequest` ([#1579](https://github.com/MetaMask/core/pull/1579)) | ||
|
|
||
| ### Removed | ||
| - **BREAKING**: Remove `fetchAll` method ([#1579](https://github.com/MetaMask/core/pull/1579)) |
There was a problem hiding this comment.
Here's an attempt to explain in more detail how this breaking change should be handled:
| - **BREAKING**: Remove `fetchAll` method ([#1579](https://github.com/MetaMask/core/pull/1579)) | |
| - **BREAKING**: Remove `fetchAll` method ([#1579](https://github.com/MetaMask/core/pull/1579)) | |
| - This method was used to fetch transaction history from Etherscan. This is now handled automatically by the controller instead; new transactions are fetched for each new block if polling is enabled. | |
| - Polling can be enabled or disabled by calling `startIncomingTransactionPolling` or `stopIncomingTransactionPolling` respectively. | |
| - An immediate update can be requested by calling `updateIncomingTransactions` | |
| - The new constructor parameter `incomingTransactions.isEnabled` controls whether incoming transactions are enabled or not upon construction. |
|
|
||
| ### Removed | ||
| - **BREAKING**: Remove `fetchAll` method ([#1579](https://github.com/MetaMask/core/pull/1579)) | ||
| - **BREAKING**: Remove `prepareUnsignedEthTx` and `getCommonConfiguration` methods ([#1581](https://github.com/MetaMask/core/pull/1581)) |
There was a problem hiding this comment.
Additional detail about the change:
| - **BREAKING**: Remove `prepareUnsignedEthTx` and `getCommonConfiguration` methods ([#1581](https://github.com/MetaMask/core/pull/1581)) | |
| - **BREAKING**: Remove `prepareUnsignedEthTx` and `getCommonConfiguration` methods ([#1581](https://github.com/MetaMask/core/pull/1581)) | |
| - - These methods were intended mainly for internal use, so it's likely this change doesn't affect most projects |
There was a problem hiding this comment.
This particular line seems like it takes more than it gives and the changelog can do without it?
There was a problem hiding this comment.
Done.
I've left the additional line as it does indeed add some noise but I think the relief and info for the future adopter is by far the lesser of two evils.
(We're also adopting this in the clients but I think it's a great standard to follow)
There was a problem hiding this comment.
Yeah I was on the fence about it as well! But this seems OK for now.
It felt wrong to leave public changes undocumented, even if we weren't aware of any usage of them. But it also felt misleading to list them here with no context, as reviewers might not realize that it doesn't impact them.
|
Looks good! Had a few changelog suggestions, let me know what you think. Lots of pending changes in other packages, but I did verify that this package doesn't depend on any of them. The changes to immediate dependencies are all due to #1622 which doesn't affect the published packages. |
Thanks Mark, I've done all your recommended changes, much more info and will add more detail for breaking changes going forward. Concerning other packages, I always do a manual diff of each other package before ignoring them to ensure no dependencies 👍 |
5 participants
Release new version of
@metamask/transaction-controller.Added
baseFeePerGasto transaction metadata (#1590)txReceiptto transaction metadata (#1592)initApprovalsmethod to generate approval requests from unapproved transactions (#1575)dappSuggestedGasFeesto transaction metadata (#1617)incomingTransactionsconstructor arguments (#1579)apiKeyincludeTokenTransfersisEnabledupdateTransactionsstartIncomingTransactionPollingstopIncomingTransactionPollingupdateIncomingTransactionsrequireApprovaloption toaddTransactionmethod options (#1580)addressargument towipeTransactionsmethod (#1573)Changed
getSelectedAddresscallback argument to constructor (#1579)isSupportedNetworkmethod toRemoteTransactionSourceinterface (#1579)addTransactionmethod (#1576)RemoteTransactionSourceRequesttype (#1579)fromBlockproperty has changed fromstringtonumbernetworkTypeproperty has been removedRemoved
fetchAllmethod (#1579)startIncomingTransactionPollingorstopIncomingTransactionPollingrespectivelyupdateIncomingTransactionsincomingTransactions.isEnabledacts as an override to disable this functionality based on a client preference for exampleprepareUnsignedEthTxandgetCommonConfigurationmethods (#1581)