Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

[:unicorn: Uniswap Merge] - imports delight #1982

Merged
merged 1,168 commits into from
Jan 3, 2022
Merged

[:unicorn: Uniswap Merge] - imports delight #1982

merged 1,168 commits into from
Jan 3, 2022

Conversation

nenadV91
Copy link
Contributor

@nenadV91 nenadV91 commented Dec 8, 2021

Summary

Closes our commander Uniswap backmerge task

Updates us to v4.20.2

Test

🔭 GP Swap: Gnosis Protocol v2 Swap UI

Tasks

  • Merge and resolve conflicts
  • Fix issues with styles in modals
  • Fix issue with missing wallet icons in header
  • Implement new NetworkSelector component

Overview

WIP

zzmp and others added 30 commits August 19, 2021 16:44
* chore: change language icon to globe
redux-devtools-extension integration is already provided through @reduxjs/toolkit.
* fix: remove the walletconnect bridge url

* fix: use the latest ethereum provider which fixes the optimism rainbow issue
reconfigure service workers to only cache used assets (excluding .po language files and non-.var.woff2 font files)

* fix: turn service workers back on

* chore: configure service worker caches

* chore: add newline

* Fix code style issues with ESLint

* chore: limit service-worker caching

Co-authored-by: Zach Pomerantz <[email protected]>
Co-authored-by: Lint Action <[email protected]>
* upgrade to 7.0

* first iteration of stubbing subgraph in integration tests

* added fixtures

* add tests for fee tier distribution and liquidity chart

* remove unused test utils

* update yarn.lock

* fixed merge artifacts
* fix(governance): special case bravo parsing

* fix(governance): explicitly parse U+2018, U+2026

* chore: upgrade ethers to ^5.4.6

Updates parseLog to delay parsing nested properties, so that they will only fail when accessed.

* fix(vote): recover from invalid UTF-8 in proposal descriptions

* fix(vote): special case bravo proposal newlines

* chore: rm dead reference
Bumps [tar](https://github.com/npm/node-tar) from 6.1.4 to 6.1.11.
- [Release notes](https://github.com/npm/node-tar/releases)
- [Changelog](https://github.com/npm/node-tar/blob/main/CHANGELOG.md)
- [Commits](isaacs/node-tar@v6.1.4...v6.1.11)

---
updated-dependencies:
- dependency-name: tar
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix: Object.fromEntries polyfill

* fix: update polyfills

* fix: update polyfills
* chore: upgrade type system

Upgrade types, typescript, and formatters.

* fix: ListsState tests
Adds a en-space to StyledPollingNumber so it is always painted. Fixes a layout shift when rendering the first block number.
StyledPollingNumber is taller than its container, so waiting to paint it causes a layout shift when rendering.
Avoids a flash of "Select a token" as the App initializes.
* first pass

* Fix code style issues with ESLint

* address comments

Co-authored-by: Lint Action <[email protected]>
commit 5f1d1af62bcd47286aafacc18788f4e7e22dd7c0
Author: Aseem Sood <[email protected]>
Date:   Fri Sep 3 12:12:39 2021 -0400

    update readme

    tweak

commit 80a5b95c0e0ae8934b5591c982077eaa813db747
Author: Noah Zinsmeister <[email protected]>
Date:   Fri Sep 3 11:52:56 2021 -0400

    consolidate and standardize unsupported tokens
Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Reviewed files from 202 to 314

  • custom/utils/blocknative - check whether they export their typings
  • src/index

@@ -85,7 +85,7 @@ import { formatSmart } from 'utils/format'
import { RowSlippage } from 'components/swap/TradeSummary/RowSlippage'
import usePrevious from 'hooks/usePrevious'
import { StyledAppBody } from './styleds'
import { ApplicationModal } from 'state/application/actions'
import { ApplicationModal } from '@src/state/application/reducer'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have to import from @src? In other places you didn't
Uploading Screenshot from 2021-12-16 14-05-03.png…

@@ -37,7 +38,7 @@ import TradeGp from './TradeGp'
import { SupportedChainId as ChainId } from 'constants/chains'
import { WETH9_EXTENDED as WETH, GpEther as ETHER } from 'constants/tokens'

import { BAD_RECIPIENT_ADDRESSES } from 'state/swap/hooks'
import { BAD_RECIPIENT_ADDRESSES } from '@src/state/swap/hooks'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have to use @src?

@@ -422,10 +428,10 @@ export function queryParametersToSwapState(parsedQs: ParsedQs, defaultInputCurre

return {
[Field.INPUT]: {
currencyId: inputCurrency,
currencyId: inputCurrency === '' ? null : inputCurrency ?? null,
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this is coming from the merge but, hum?

Am I reading this wrong or is this equivalent to inputCurrecy || null ?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure seems loik it

@@ -114,7 +115,7 @@ const PERMIT_ALLOWED_TYPE = [
{ name: 'allowed', type: 'bool' },
]

export function useERC20Permit(
function useERC20Permit(
Copy link
Contributor

Choose a reason for hiding this comment

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

Will probably need the export?

import { useAppDispatch } from 'state/hooks'

import { getNetworkLibrary } from '../connectors'
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep absolute import?

@@ -1,6 +1,7 @@
import { Currency, Token } from '@uniswap/sdk-core'
import { useMemo } from 'react'
import { useUnsupportedTokens } from 'hooks/Tokens'

import { useUnsupportedTokens } from './Tokens'
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep absolute import?

function Updaters() {
return (
<>
<RadialGradientByChainUpdater />
<ListsUpdater />
<UserUpdater />
<ApplicationUpdater />
<TransactionUpdater />
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep this updater?
Maybe we already disabled, but should check

Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Checked the final ~100 files

  • state/application/actions - make sure behaviour is still needed and captured elsewhere
  • state/transactions/actions - new transaction types
  • utils/addnetwork - is it useful for us?
  • utils/gettokenlists - lazy loading dependencies, we could reuse it for other stuff

@@ -6,10 +6,10 @@ import AddressClaimModal from '../components/claim/AddressClaimModal'
import ErrorBoundary from '../components/ErrorBoundary'
import Header from '../components/Header'
import Polling from '../components/Header/Polling'
import Popups from 'components/Popups'
import Popups from '../components/Popups'
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep import

import UnsupportedCurrencyFooter from 'components/swap/UnsupportedCurrencyFooter'
import { MouseoverTooltip, MouseoverTooltipContent } from 'components/Tooltip'
import { MouseoverTooltip, MouseoverTooltipContent } from '@src/components/Tooltip'
Copy link
Contributor

Choose a reason for hiding this comment

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

Need @src?

import AddressInputPanel from '../../components/AddressInputPanel'
import { ButtonConfirmed, ButtonError, ButtonGray, ButtonLight, ButtonPrimary } from 'components/Button'
import { ButtonConfirmed, ButtonError, ButtonLight, ButtonPrimary } from '../../components/Button'
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably keep imports

import { useActiveWeb3React } from 'hooks/web3'
import { AppState } from 'state'
import { addPopup, ApplicationModal, PopupContent, removePopup, setOpenModal } from 'state/application/actions'
import { useAppDispatch, useAppSelector } from '@src/state/hooks'
Copy link
Contributor

Choose a reason for hiding this comment

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

Need @src?

import { useAppDispatch, useAppSelector } from '@src/state/hooks'

import { useActiveWeb3React } from '../../hooks/web3'
import { AppState } from '../index'
Copy link
Contributor

Choose a reason for hiding this comment

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

Relative imports

"baseUrl": "src"
},
"exclude": [
"node_modules",
"cypress"
],
"include": [
"src"
"src/**/*"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need this?

@nenadV91
Copy link
Contributor Author

I've adjusted react-cosmos config so its working in our app and also added examples for 2 components. To use this you can run yarn run cosmos and it will start a server on http://localhost:5000/ where you can preview the components. Basically to add some component to cosmos you create a fixtures folder or a file with .fixture.tsx name. I don't know if we want to use it or not but now we can and know how.

react-compass

@elena-zh
Copy link

Hey @nenadV91 , great job!
The vast majority of issues is fixed now.

Remaining:

  1. It would be great to remove 'Auto Router' from the settings in a mobile view
    image

  2. Case with network switching in mobile MM integrated browser is still here: The app seems to be disconnected when switch a network in MM injected browser. So it would be great to force the app reloading after a network switching. But let me know if I need to create a low-prio separate issue for this

disc.mp4
  1. Just to be on the same page for the network switcher. Currently, it works great besides the part of the logic when we are connected to a wallet using WC: the switcher is disabled in this case, Should we leave it as it is? @anxolin , @biocom , @alfetopito ?
    disabled

@elena-zh
Copy link

Case with network switching in mobile MM integrated browser is still here: The app seems to be disconnected when switch a network in MM injected browser. So it would be great to force the app reloading after a network switching. But let me know if I need to create a low-prio separate issue for this

Actually, I have created #2019 bug as it is not only related to MM wallet. Besides, the issue is reproducible on Prod now.

@alfetopito
Copy link
Contributor

It would be great to remove 'Auto Router' from the settings in a mobile view

Auto router section should be hidden everywhere as we do not have that option on our side.

@nenadV91
Copy link
Contributor Author

Auto-router is removed now

Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Few more things, ignore if you already checked and they are not needed.

@@ -16,7 +17,7 @@ export const StyledClose = styled(X)`
cursor: pointer;
}
`
export const Popup = styled.div`
const Popup = styled.div`
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep this one also?

Comment on lines 15 to 16
import { ExternalLink } from '../../theme'
import { CloseIcon, CustomLightSpinner } from '../../theme/components'
Copy link
Contributor

Choose a reason for hiding this comment

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

Import on these 2 lines

Comment on lines 7 to 11
import { TYPE } from 'theme'
import { TYPE } from '../../theme'
Copy link
Contributor

Choose a reason for hiding this comment

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

import

import { Percent } from '@uniswap/sdk-core'
import { L2_CHAIN_IDS } from '@src/constants/chains'
Copy link
Contributor

Choose a reason for hiding this comment

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

@src

import { IdenticonProps } from 'components/Identicon'

import { useActiveWeb3React } from '../../hooks/web3'
Copy link
Contributor

Choose a reason for hiding this comment

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

import

}, */
[SupportedChainId.XDAI]: {
docs: 'https://docs.uniswap.org/',
explorer: 'https://gnosis-protocol.io/xdai',
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this was there already, but the explorer should be BlockScout rather than our explorer


// SDN OFAC addresses
const BLOCKED_ADDRESSES: string[] = [
'0x7Db418b5D567A4e0E8c59Ad71BE1FcE48f3E6107',
Copy link
Contributor

Choose a reason for hiding this comment

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

make a note of these blocked addresses please

@@ -284,7 +290,7 @@ export default function Header() {
>
<Trans>Pool</Trans>
</StyledNavLink>
{chainId && chainId === SupportedChainId.MAINNET && (
{(!chainId || chainId === SupportedChainId.MAINNET) && (
Copy link
Contributor

Choose a reason for hiding this comment

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

so no it's if you're not connected or you're on mainnet?

@W3stside
Copy link
Contributor

after some minor import changes, looks good to me now, @gnosis/gp-qa anything holding this back?

@elena-zh
Copy link

@W3stside , @nenadV91 the latest changes LGTM!
The only thing remains is to agree on this point:

Just to be on the same page for the network switcher. Currently, it works great besides the part of the logic when we are connected to a wallet using WC: the switcher is disabled in this case, Should we leave it as it is?
disabled

@W3stside W3stside self-requested a review December 24, 2021 12:11
Copy link
Contributor

@W3stside W3stside left a comment

Choose a reason for hiding this comment

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

Very nice work. Take a break form this for a while 😅

@nenadV91 nenadV91 merged commit b85ca42 into develop Jan 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.