From ca62b14258c53cb361a720a8ad38b6948e8d6c49 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 4 Jul 2019 13:25:36 -0300 Subject: [PATCH] Declare variables before use While working on #6805, I noticed that many variables were being used before they were declared. Technically this worked fine in practice because we were using the `transform-es2015-block-scoping` Babel plugin, which transforms `let` and `const` to `var`, which is hoisted. However, after removing that Babel transformation, many things broke. All instances of variables or classes being used before declared have been fixed. The `no-use-before-define` eslint rule has been added to catch these cases going forward. The rule is disabled for function declarations for the moment, because those are always hoisted. We could disable that too if we want to, but it's purely stylistic and would require a lot more changes. --- .eslintrc | 1 + app/scripts/inpage.js | 89 ++++++++++--------- app/scripts/lib/createStreamSink.js | 13 ++- .../controllers/metamask-controller-test.js | 25 +++--- test/unit/ui/app/reducers/app.spec.js | 2 +- ui/app/helpers/utils/conversion-util.js | 14 +-- ui/app/pages/routes/index.js | 24 ++--- ui/app/store/actions.js | 28 +++--- 8 files changed, 99 insertions(+), 97 deletions(-) diff --git a/.eslintrc b/.eslintrc index 53033b753bc9..22585aa9dfe1 100644 --- a/.eslintrc +++ b/.eslintrc @@ -134,6 +134,7 @@ "no-unreachable": 2, "no-unsafe-finally": 2, "no-unused-vars": [2, { "vars": "all", "args": "all", "argsIgnorePattern": "[_]+" }], + "no-use-before-define": [2, { "functions": false }], "no-useless-call": 2, "no-useless-computed-key": 2, "no-useless-constructor": 2, diff --git a/app/scripts/inpage.js b/app/scripts/inpage.js index fb16dd43d736..2a0a9ad18250 100644 --- a/app/scripts/inpage.js +++ b/app/scripts/inpage.js @@ -1,4 +1,36 @@ /*global Web3*/ + + +// need to make sure we aren't affected by overlapping namespaces +// and that we dont affect the app with our namespace +// mostly a fix for web3's BigNumber if AMD's "define" is defined... +let __define + +/** + * Caches reference to global define object and deletes it to + * avoid conflicts with other global define objects, such as + * AMD's define function + */ +const cleanContextForImports = () => { + __define = global.define + try { + global.define = undefined + } catch (_) { + console.warn('MetaMask - global.define could not be deleted.') + } +} + +/** + * Restores global define object from cached reference + */ +const restoreContextAfterImports = () => { + try { + global.define = __define + } catch (_) { + console.warn('MetaMask - global.define could not be overwritten.') + } +} + cleanContextForImports() require('web3/dist/web3.min.js') const log = require('loglevel') @@ -46,6 +78,20 @@ inpageProvider.enable = function ({ force } = {}) { // this will be default true so it does not break any old apps. inpageProvider.autoRefreshOnNetworkChange = true + +// publicConfig isn't populated until we get a message from background. +// Using this getter will ensure the state is available +const getPublicConfigWhenReady = async () => { + const store = inpageProvider.publicConfigStore + let state = store.getState() + // if state is missing, wait for first update + if (!state.networkVersion) { + state = await new Promise(resolve => store.once('update', resolve)) + console.log('new state', state) + } + return state +} + // add metamask-specific convenience methods inpageProvider._metamask = new Proxy({ /** @@ -87,19 +133,6 @@ inpageProvider._metamask = new Proxy({ }, }) -// publicConfig isn't populated until we get a message from background. -// Using this getter will ensure the state is available -async function getPublicConfigWhenReady () { - const store = inpageProvider.publicConfigStore - let state = store.getState() - // if state is missing, wait for first update - if (!state.networkVersion) { - state = await new Promise(resolve => store.once('update', resolve)) - console.log('new state', state) - } - return state -} - // Work around for web3@1.0 deleting the bound `sendAsync` but not the unbound // `sendAsync` method on the prototype, causing `this` reference issues with drizzle const proxiedInpageProvider = new Proxy(inpageProvider, { @@ -161,33 +194,3 @@ inpageProvider.publicConfigStore.subscribe(function (state) { window.postMessage('onboardingcomplete', '*') } }) - -// need to make sure we aren't affected by overlapping namespaces -// and that we dont affect the app with our namespace -// mostly a fix for web3's BigNumber if AMD's "define" is defined... -let __define - -/** - * Caches reference to global define object and deletes it to - * avoid conflicts with other global define objects, such as - * AMD's define function - */ -function cleanContextForImports () { - __define = global.define - try { - global.define = undefined - } catch (_) { - console.warn('MetaMask - global.define could not be deleted.') - } -} - -/** - * Restores global define object from cached reference - */ -function restoreContextAfterImports () { - try { - global.define = __define - } catch (_) { - console.warn('MetaMask - global.define could not be overwritten.') - } -} diff --git a/app/scripts/lib/createStreamSink.js b/app/scripts/lib/createStreamSink.js index b93dbc0893c3..eb9d0bd1ac79 100644 --- a/app/scripts/lib/createStreamSink.js +++ b/app/scripts/lib/createStreamSink.js @@ -1,13 +1,6 @@ const WritableStream = require('readable-stream').Writable const promiseToCallback = require('promise-to-callback') -module.exports = createStreamSink - - -function createStreamSink (asyncWriteFn, _opts) { - return new AsyncWritableStream(asyncWriteFn, _opts) -} - class AsyncWritableStream extends WritableStream { constructor (asyncWriteFn, _opts) { @@ -22,3 +15,9 @@ class AsyncWritableStream extends WritableStream { } } + +function createStreamSink (asyncWriteFn, _opts) { + return new AsyncWritableStream(asyncWriteFn, _opts) +} + +module.exports = createStreamSink diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js index 1b8cde42e6f6..e8e407a7fc10 100644 --- a/test/unit/app/controllers/metamask-controller-test.js +++ b/test/unit/app/controllers/metamask-controller-test.js @@ -679,19 +679,6 @@ describe('MetaMaskController', function () { }) describe('#newUnsignedPersonalMessage', function () { - - it('errors with no from in msgParams', async () => { - const msgParams = { - 'data': data, - } - try { - await metamaskController.newUnsignedPersonalMessage(msgParams) - assert.fail('should have thrown') - } catch (error) { - assert.equal(error.message, 'MetaMask Message Signature: from field is required.') - } - }) - let msgParams, metamaskPersonalMsgs, personalMessages, msgId const address = '0xc42edfcc21ed14dda456aa0756c153f7985d8813' @@ -718,6 +705,18 @@ describe('MetaMaskController', function () { personalMessages[0].msgParams.metamaskId = parseInt(msgId) }) + it('errors with no from in msgParams', async () => { + const msgParams = { + 'data': data, + } + try { + await metamaskController.newUnsignedPersonalMessage(msgParams) + assert.fail('should have thrown') + } catch (error) { + assert.equal(error.message, 'MetaMask Message Signature: from field is required.') + } + }) + it('persists address from msg params', function () { assert.equal(metamaskPersonalMsgs[msgId].msgParams.from, address) }) diff --git a/test/unit/ui/app/reducers/app.spec.js b/test/unit/ui/app/reducers/app.spec.js index 09cf3dbf007b..28f5ebb33a0f 100644 --- a/test/unit/ui/app/reducers/app.spec.js +++ b/test/unit/ui/app/reducers/app.spec.js @@ -700,7 +700,7 @@ describe('App State', () => { }) it('hides sub loading indicator', () => { - const oldState = {...metamaskState, ...oldState} + const oldState = {...metamaskState, isSubLoading: true } const state = reduceApp(oldState, { type: actions.HIDE_SUB_LOADING_INDICATION, }) diff --git a/ui/app/helpers/utils/conversion-util.js b/ui/app/helpers/utils/conversion-util.js index affddade7bac..46bcfe47b066 100644 --- a/ui/app/helpers/utils/conversion-util.js +++ b/ui/app/helpers/utils/conversion-util.js @@ -37,13 +37,6 @@ const BIG_NUMBER_WEI_MULTIPLIER = new BigNumber('1000000000000000000') const BIG_NUMBER_GWEI_MULTIPLIER = new BigNumber('1000000000') const BIG_NUMBER_ETH_MULTIPLIER = new BigNumber('1') -// Individual Setters -const convert = R.invoker(1, 'times') -const round = R.invoker(2, 'round')(R.__, BigNumber.ROUND_HALF_DOWN) -const roundDown = R.invoker(2, 'round')(R.__, BigNumber.ROUND_DOWN) -const invertConversionRate = conversionRate => () => new BigNumber(1.0).div(conversionRate) -const decToBigNumberViaString = () => R.pipe(String, toBigNumber['dec']) - // Setter Maps const toBigNumber = { hex: n => new BigNumber(stripHexPrefix(n), 16), @@ -66,6 +59,13 @@ const baseChange = { BN: n => new BN(n.toString(16)), } +// Individual Setters +const convert = R.invoker(1, 'times') +const round = R.invoker(2, 'round')(R.__, BigNumber.ROUND_HALF_DOWN) +const roundDown = R.invoker(2, 'round')(R.__, BigNumber.ROUND_DOWN) +const invertConversionRate = conversionRate => () => new BigNumber(1.0).div(conversionRate) +const decToBigNumberViaString = () => R.pipe(String, toBigNumber['dec']) + // Predicates const fromAndToCurrencyPropsNotEqual = R.compose( R.not, diff --git a/ui/app/pages/routes/index.js b/ui/app/pages/routes/index.js index afef0692ed9c..c292fd9c737c 100644 --- a/ui/app/pages/routes/index.js +++ b/ui/app/pages/routes/index.js @@ -183,18 +183,6 @@ class Routes extends Component { this.getConnectingLabel(loadingMessage) : null log.debug('Main ui render function') - const sidebarOnOverlayClose = sidebarType === WALLET_VIEW_SIDEBAR - ? () => { - this.context.metricsEvent({ - eventOpts: { - category: 'Navigation', - action: 'Wallet Sidebar', - name: 'Closed Sidebare Via Overlay', - }, - }) - } - : null - const { isOpen: sidebarIsOpen, transitionName: sidebarTransitionName, @@ -203,6 +191,18 @@ class Routes extends Component { } = sidebar const { transaction: sidebarTransaction } = props || {} + const sidebarOnOverlayClose = sidebarType === WALLET_VIEW_SIDEBAR + ? () => { + this.context.metricsEvent({ + eventOpts: { + category: 'Navigation', + action: 'Wallet Sidebar', + name: 'Closed Sidebare Via Overlay', + }, + }) + } + : null + return (
{ + log.debug(`background.getState`) + + return new Promise((resolve, reject) => { + background.getState((error, newState) => { + if (error) { + return reject(error) + } + + resolve(newState) + }) + }) +} + function updateTransaction (txData) { log.info('actions: updateTx: ' + JSON.stringify(txData)) return dispatch => { @@ -1618,20 +1632,6 @@ const backgroundSetLocked = () => { }) } -const updateMetamaskStateFromBackground = () => { - log.debug(`background.getState`) - - return new Promise((resolve, reject) => { - background.getState((error, newState) => { - if (error) { - return reject(error) - } - - resolve(newState) - }) - }) -} - function lockMetamask () { log.debug(`background.setLocked`)