feat: add @fuel-ts/utils/assets-utils package
#1703
Conversation
| return CHAIN_IDS.eth.sepolia; | ||
| } | ||
| if (networkType === 'fuel') { | ||
| return CHAIN_IDS.fuel.beta4; |
There was a problem hiding this comment.
This PR should probably target beta5 already.
| "./assets-utils": { | ||
| "require": "./dist/assets-utils.js", | ||
| "import": "./dist/assets-utils.mjs", | ||
| "types": "./dist/assets-utils.d.ts" |
There was a problem hiding this comment.
I wonder if it should be inside the new account package, following the Provider route:
cc @Torres-ssf
There was a problem hiding this comment.
@arboleya Makes sense. Do you believe I should wait for that PR to be merged, or can I work on this in parallel?
There was a problem hiding this comment.
There are things you could do ahead of time, or you could also branch off of that branch/PR.
That's entirely up to you.
There was a problem hiding this comment.
@arboleya I agree, this can be part of the account package.
There was a problem hiding this comment.
Maybe it could even go inside providers:
account/src/providers/chains.tsaccount/src/providers/assets/index.tsaccount/src/providers/assets/icons/..
There was a problem hiding this comment.
why inside providers ? assets has nothing to do with network/account... they can be even from other chain
I may lack some context here but @fuel-ts/assets feels good to me cc @arboleya
There was a problem hiding this comment.
I agree; however, over-categorizing small chunks of code led us to have dozens of packages flattened out in the repo root without an apparent relationship between them, which created a maintenance burden we are trying to reduce by re-grouping things in this PR as an attempt to defrag the list of packages. Things might not look perfect and are subject to change, but we must start somewhere, and flexibility is required to get things moving.
Nevertheless, everything will still be available from fuels.
|
|
||
| export const assets = assetList; | ||
|
|
||
| export default resolveIconPath(process.env.BASE_URL || './', assetList); |
There was a problem hiding this comment.
I usually dislike export default since it can result in inconsistent nomenclatures.
There was a problem hiding this comment.
Agreed, and I don't think we use default exports anywhere at all. I kept it there because for now I just copy pasted all the source from the old repo, but happy to make this change
| "@fuel-ts/utils": minor | ||
| --- | ||
|
|
||
| add @fuel-ts/utils/assets-utils package |
There was a problem hiding this comment.
It looks more like a directory/script. 🤔
There was a problem hiding this comment.
Hmm.. thought of it as a package/mini-package since I put it under a separate export path.
Directory makes sense tho lol!
There was a problem hiding this comment.
After you move it around, the description may naturally morph with it.
| ### Usage | ||
|
|
||
| ```ts | ||
| import assets from "@fuel-ts/utils/assets-utils"; |
There was a problem hiding this comment.
We might not need an entry point, even if we don't move assets inside account.
import { assets } from "@fuel-ts/utils";
There was a problem hiding this comment.
friendly reminder to add index file under utils directory, just like this pr -> https://github.com/FuelLabs/fuels-npm-packs/pull/41/files
Closes #1541