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

chore(account): fixed TypeError for accounts with 3+ tokens #455

Merged
merged 3 commits into from
Aug 18, 2018

Conversation

mhuggins
Copy link
Contributor

@mhuggins mhuggins commented Aug 18, 2018

Description

This fixes an issue with the distributable version (via yarn dist) that caused the app crash when opening the Account screen.

Motivation and Context

The app crashes when trying to render the breakdown chart with 3+ tokens. The error is:

TypeError: Assignment to constant variable.

This is caused by the uglify plugin inlining single-use functions, as discovered here: mishoo/UglifyJS#2842 (comment)

How Has This Been Tested?

  1. Use yarn dist to create a distributable version of the app.
  2. Run the app, and login with an account that has 3+ distinct token balances.
  3. Observe the app "crash" (HTML output disappears).

Types of changes

  • Chore (tests, refactors, and fixes)
  • New feature (adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have read the CONTRIBUTING guidelines and confirm that my code follows the code style of this project.
  • Tests for the changes have been added (for bug fixes/features)

Documentation

  • Docs need to be added/updated (for bug fixes/features)

Closing issues

N/A

@mhuggins mhuggins added the PR: needs review Pull request label Aug 18, 2018
@codecov
Copy link

codecov bot commented Aug 18, 2018

Codecov Report

Merging #455 into develop will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #455      +/-   ##
===========================================
+ Coverage    53.08%   53.13%   +0.04%     
===========================================
  Files          153      153              
  Lines         1230     1229       -1     
  Branches       161      161              
===========================================
  Hits           653      653              
+ Misses         486      485       -1     
  Partials        91       91
Impacted Files Coverage Δ
...unt/components/AccountPanel/Breakdown/Breakdown.js 90.9% <100%> (+7.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e78d20...c89136f. Read the comment docs.

UglifyJsPlugin will inline single-use functions by default.  This can result in an error
stating `TypeError: Assignment to constant variable`.  To work around this, the function
is reused since there was an opportunity to do so anyway.

mishoo/UglifyJS#2842 (comment)
@DalderupMaurice
Copy link
Member

I don't have an account with 3+ tokens 😂😅

@mhuggins
Copy link
Contributor Author

PM'd you an account to test this out on.

@DalderupMaurice DalderupMaurice merged commit 3172934 into develop Aug 18, 2018
@DalderupMaurice DalderupMaurice deleted the chore/type-error branch August 18, 2018 14:42
@DalderupMaurice DalderupMaurice added PR: good to merge Reviewed and approved and removed PR: needs review Pull request labels Aug 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: good to merge Reviewed and approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants