Skip to content

Remove BN.js from web/mobile/common/embed/fixed-decimal - #12368

Merged
dharit-tan merged 19 commits into
mainfrom
rt-fixed-decimal-2
Jun 23, 2025
Merged

Remove BN.js from web/mobile/common/embed/fixed-decimal#12368
dharit-tan merged 19 commits into
mainfrom
rt-fixed-decimal-2

Conversation

@dharit-tan

@dharit-tandharit-tan commented Jun 20, 2025

Copy link
Copy Markdown
Contributor

Description

Death to BN.js!!!

Also cleaned up some dead code from the old PayAndEarnPage stuff.

How Has This Been Tested?

Tested locally on web + mobile stage, but probably needs more testing

@changeset-bot

changeset-botBot commented Jun 20, 2025•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 54e85ee

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
NameType
@audius/fixed-decimalMinor
@audius/sdkPatch
@audius/sdk-legacyPatch
@audius/sp-actionsPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@socket-security

socket-securityBot commented Jun 20, 2025•
edited
Loading

Copy link
Copy Markdown

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

dylanjeffers
dylanjeffers approved these changes Jun 20, 2025
View reviewed changes

@dylanjeffersdylanjeffers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sweet lets send it! note do you want this to be included in release next week or would you rather have a week of qa on staging?

rickyrombo
rickyrombo approved these changes Jun 20, 2025
View reviewed changes

@rickyromborickyrombo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR makes my heart smile! ❤️

Comment threadpackages/common/src/api/tan-query/jupiter/optimisticUpdates.ts Outdated
Comment threadpackages/common/src/api/tan-query/jupiter/utils.ts

try {
const usdcValue = USDC(new BN(weiValue)).floor(2)
const usdcValue = USDC(BigInt(weiValue) as UsdcWei).floor(2)

@rickyromborickyromboJun 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(can't comment above) wondering if we even need formatUSDCValue? The default toLocaleString() for USDC should hopefully encompass a lot of what we're doing here.

@dharit-tandharit-tanJun 23, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

hmmm that's a bigger q, @faridsalau thoughts?

@faridsalaufaridsalauJun 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yea it has a few relevant checks, can probably slim down, but wouldn't touch for now

@rickyromborickyromboJun 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What are the relevant checks? I'm not totally convinced that the callers of this shouldn't be using FixedDecimal directly

Comment threadpackages/common/src/hooks/useAudioBalance.ts
Comment threadpackages/common/src/hooks/useFormattedAudioBalance.ts
? AUDIO(totalBalance).toLocaleString('en-US', {
maximumFractionDigits: decimalPlaces
})
? formatNumberCommas(

@rickyromborickyromboJun 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we need ``formatNumberCommas- tryuseGrouping: true` https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat#usegrouping

@rickyromborickyromboJun 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

useGrouping should actually be true by default, is that not what you're seeing?

@dharit-tandharit-tanJun 23, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

it was working totally fine in web but for some reason no commas on mobile, so i added this

@rickyromborickyromboJun 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ahhh interesting!! I wonder if the JS engine RN uses doesn't implement toLocaleString completely...

Comment threadpackages/common/src/hooks/usePurchaseMethod.ts Outdated
Comment threadpackages/web/src/components/buy-audio-modal/components/SuccessPage.tsx Outdated
Comment threadpackages/web/src/components/buy-audio-modal/components/SuccessPage.tsx
Comment threadpackages/web/src/components/withdraw-usdc-modal/components/TransferSuccessful.tsx Outdated
Comment threadpackages/web/src/pages/audio-page/components/modals/AudioBreakdownModal.tsx Outdated
Comment threadpackages/web/src/pages/pay-and-earn-page/components/WithdrawalsTab.tsx Outdated
@dharit-tan
dharit-tan merged commit 1026ce1 into mainJun 23, 2025
@dharit-tan
dharit-tan deleted the rt-fixed-decimal-2 branch June 23, 2025 19:07
faridsalau pushed a commit that referenced this pull request Jun 24, 2025
@dharit-tan@faridsalau
Remove BN.js from web/mobile/common/embed/fixed-decimal (#12368)
c55467c
dylanjeffers added a commit that referenced this pull request Jun 25, 2025
@dylanjeffers
Revert "Remove BN.js from web/mobile/common/embed/fixed-decimal (#12368…
68e058b
…)"
This reverts commit c55467c.
audius-infra pushed a commit that referenced this pull request Jun 28, 2025
audius-infra
Audius Client (Web and Mobile) v1.5.141
40fc921
[f1cd30c] [PE-6452] Refactor web lineup tiles (#12424) Dylan Jeffers
[b83ee65] Prime search results to improve autocomplete performance (#12425) Isaac Solo
[c12b5d7] Add rich notifications to enable image urls (#12428) Farid Salau
[fe71b33] Add analytics to Buy/Sell flow (#12415) Farid Salau
[c006e1f] Move protocol dashboard to cloudflare deployment (#12420) Ray Jacobson
[d1b2ba1] [HOTFIX] Fix manager mode not letting you swap from managee (#12423) JD Francis
[1e34849] [PE-6454] Fix purchases after BN.js migration (#12426) Reed
[6b465b8] Use prime functions for entity updates (#12421) Dylan Jeffers
[c428cd2] Drop search test (#12422) Dylan Jeffers
[31f5416] Fix another incorrect instance of waitForValue (#12419) JD Francis
[79e3551] Fix purchase flow stuck due to waitForValue (#12418) JD Francis
[1717738] [QA-2184] Fix double $ signs in purchase flow (#12417) Reed
[ca3a6a2] Fix publish playlist button on missing track count (#12413) Isaac Solo
[e6d86ba] Fix AudioHoverCard balance (#12416) Reed
[3b2fecf] Update AudioBreakdownDrawer to use balance hooks and harmony (#12414) Reed
[35a55b1] [PE-6450] Fix mobile audio balance formatting (#12412) Reed
[6bc66da] Fix search lineup issues (#12411) JD Francis
[bf7fa37] [QA-2168] Fix mobile-web prev/next button color (#12408) Dylan Jeffers
[b9e1b25] [PE-6448] Fix mobile track tile favorites and library loading (#12409) Dylan Jeffers
[d9c33bf] Fix infinite loop in mobile DMs (#12410) JD Francis
[3e940b9] [PE-6418] Fix artist all hidden tracks (#12405) Dylan Jeffers
[d7ffb72] Fix mobile library track tab filter display (#12402) Dylan Jeffers
[11cefe8] [PE-6431] Fix extra user/handle API calls (#12406) JD Francis
[2ef0a61] Adjust content padding and center align text (#12403) Isaac Solo
[ec230ca] [PE-6433] Fix signed out search (#12400) Dylan Jeffers
[3da884f] [PE-6441] Fix create new collection (#12404) Dylan Jeffers
[e5405c6] [PE-6443] Fix collection tiles not playing in lineups (#12398) JD Francis
[09a7716] [PE-6436] Fix mobile withdraw flow scrolling and validation (#12401) Reed
[b351681] Add reactotron for RN network debugging (#12399) Farid Salau
[7898ff7] Add retries on swaps (#12397) Farid Salau
[8beb206] Fix header snapping and improve content scrolling (#12396) Isaac Solo
[ea8b873] [PE-6427] Fix play bar duration (#12389) Dylan Jeffers
[7de9a49] [QA-2160PE-6438] Fix Android icon colors (#12390) Dylan Jeffers
[2e5d6e5] [PE-6432] Reduce mobile web explore page tiles (#12393) Dylan Jeffers
[ed89aa8] [PE-6435] Fix selectable pill isSelected color (#12392) Dylan Jeffers
[c30350e] [QA-2167] Hide empty track page lineup for artists with one track (#12395) KJ
[590ccea] [QA-1625] Contain scroll in playlist tiles (#12394) KJ
[cc42ce5] [PE-6428] Fix_confetti_final_final_final_v2 (#12387) JD Francis
[8ef4ca4] [PE-6419|PE-6434] Fix flash of sign up on mobile apps due to default account status (#12384) JD Francis
[d5ba7b9] [PE-6383] Fix ATA of ATA bug in withdraw flow (#12382) Reed
[1026ce1] Remove BN.js from web/mobile/common/embed/fixed-decimal (#12368) Reed
[a539209] Optimize search and explore performance + animation (#12375) Isaac Solo
[443937c] [PE-6403] Fix mobile library radio buttons not working (#12349) JD Francis
[799cb55] [QA-2161] Fix mobile favorited album/playlist search (#12374) Dylan Jeffers
[d9bb313] [QA-2165] Fix profile nav item navigation (#12376) Dylan Jeffers
[bba94bd] [QA-2166] Fix broken mobile playback (#12377) Dylan Jeffers
[86c5fbf] Refactor TrendingPageProvider to funcitonal component (#12378) Farid Salau
[8b7db22] [QA-1985] Remove multiselect prop to fix upload tag delete on click functionality (#12379) KJ
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rickyromborickyromborickyrombo approved these changes
@dylanjeffersdylanjeffersdylanjeffers approved these changes
@faridsalaufaridsalauAwaiting requested review from faridsalau

Assignees

No one assigned

Labels

size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dharit-tan@rickyrombo@dylanjeffers@faridsalau