Skip to content

Prevent React warning for custom slippage#9706

Merged
darkwing merged 2 commits intoMetaMask:developfrom
darkwing:fix-slippage-warning
Oct 28, 2020
Merged

Prevent React warning for custom slippage#9706
darkwing merged 2 commits intoMetaMask:developfrom
darkwing:fix-slippage-warning

Conversation

@darkwing
Copy link
Copy Markdown
Contributor

Explanation:

Fixes an issue I somehow triggered by clicking in the custom slippage box quickly:

Warning: `value` prop on `input` should not be null. Consider using an empty string to clear the component or `undefined` for uncontrolled components.
    in input (created by SlippageButtons)
    in div (created by SlippageButtons)
    in button (created by ButtonGroup)
    in div (created by ButtonGroup)
    in ButtonGroup (created by SlippageButtons)
    in div (created by SlippageButtons)
    in div (created by SlippageButtons)
    in div (created by SlippageButtons)
    in SlippageButtons (created by BuildQuote)
    in div (created by BuildQuote)
    in div (created by BuildQuote)
    in div (created by BuildQuote)
    in BuildQuote (created by Context.Consumer)
    in Route (created by FeatureToggledRoute)
    in FeatureToggledRoute (created by Swap)
    in Switch (created by Swap)
    in div (created by Swap)
    in div (created by Swap)
    in div (created by Swap)
    in Swap (created by Context.Consumer)
    in Route (created by Authenticated)
    in Authenticated (created by ConnectFunction)
    in ConnectFunction (created by Routes)
    in Switch (created by Routes)
    in div (created by Routes)
    in div (created by Routes)
    in Routes (created by ConnectFunction)
    in ConnectFunction (created by Context.Consumer)
    in withRouter(Connect(Routes)) (created by Index)
    in LegacyI18nProvider (created by Index)
    in I18nProvider (created by Index)
    in LegacyMetaMetricsProvider (created by Index)
    in MetaMetricsProvider (created by Index)
    in LegacyMetaMetricsProvider (created by Index)
    in MetaMetricsProvider (created by Index)
    in Router (created by HashRouter)
    in HashRouter (created by Index)
    in Provider (created by Index)
    in Index backend.js:32
    n backend.js:32
    React 12
    unstable_runWithPriority scheduler.development.js:697
    React 26
    startApp index.js:136
    launchMetamaskUi index.js:34
    apply index.js:123
    handle index.js:99
    handle dnode.js:140
    write dnode.js:128
    ondata _stream_readable.js:612
    emitOne events.js:106
    emit events.js:184
    addChunk _stream_readable.js:284
    readableAddChunk _stream_readable.js:271
    push _stream_readable.js:238
    _write index.js:68
    doWrite _stream_writable.js:406
    writeOrBuffer _stream_writable.js:395
    write _stream_writable.js:322
    ondata _stream_readable.js:619
    emitOne events.js:106
    emit events.js:184
    addChunk _stream_readable.js:291
    readableAddChunk _stream_readable.js:278
    push _stream_readable.js:245
    _onMessage index.js:38

Manual testing steps:

  • Click in the custom slippage component, see no warnings.

@darkwing darkwing requested a review from a team as a code owner October 23, 2020 21:25
@darkwing darkwing requested a review from brad-decker October 23, 2020 21:25
@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

brad-decker
brad-decker previously approved these changes Oct 23, 2020
Gudahtt
Gudahtt previously approved these changes Oct 23, 2020
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Curious - we initialize customValue to an emptry string, but then we set the customValue to both undefined and null in different places 🤦

This approach to preventing this warning seems good to me! But we should probably align on one standard way of setting an empty value in this component.

@darkwing darkwing dismissed stale reviews from Gudahtt and brad-decker via 503ba51 October 26, 2020 13:44
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for cleaning up the other "empty" custom value cases

@darkwing darkwing merged commit 5b67cf1 into MetaMask:develop Oct 28, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Oct 28, 2020
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.

3 participants