chore: deprecate unused operation names

[Torres-ssf]

• Closes Unused OperationName Enum Entries in the SDK #3008

Checklist

• All changes are covered by tests (or not applicable)
• All changes are documented (or not applicable)
• I reviewed the entire PR myself (preferably, on GH UI)
• I described all Breaking Changes (or there's none)

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
create-fuels-template	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 3, 2024 9:52am
fuels-template	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 3, 2024 9:52am
ts-api-docs	❌ Failed (Inspect)			Oct 3, 2024 9:52am
ts-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 3, 2024 9:52am

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
create-fuels-counter-example	⬜️ Ignored (Inspect)			Oct 3, 2024 9:52am

[codspeed-hq]

CodSpeed Performance Report

Merging #3180 will improve performances by ×2.6

_{Comparing st/chore/remove-unused-operation-names (060c49f) with master (0c927f7)}

Summary

⚡ 1 improvements
✅ 20 untouched benchmarks

Benchmarks breakdown

	Benchmark	master	st/chore/remove-unused-operation-names	Change
⚡	Instantiate a new Unlocked wallet	106.1 ms	40.6 ms	×2.6

[maschad]

Although the OperationName enum is hidden it's still exported by the account package and therefore it's a breaking change no? Perhaps we should add a deprecation notice instead.

[petertonysmith94]

Although the OperationName enum is hidden it's still exported by the account package and therefore it's a breaking change no? Perhaps we should add a deprecation notice instead.

I would concur.

[Torres-ssf]

@maschad But the enum OperationName is only used internally.

This enum doesn't have any purpose for users to use it within their code. Especially the removed entries as the SDK is not even using them.

We don't even have operations that use the removed enum entries.

[Dhaiwat10]

@maschad Is it practically a breaking change though? Like Sergio mentioned, there is no use of this enum for anyone really

[maschad]

The issue here is that we can't guarantee that a consumer downstream didn't use this enum in their code since it was exported and as such if we remove properties from it then it would break their code.

[arboleya]

@maschad This seems like junk code that doesn't fit and can't be used anywhere - or can it?

@Torres-ssf I believe we had another PR like this a few weeks ago, removing some broken/obsolete code. I couldn't find it, maybe you remember something.

[maschad]

@arboleya I believe you may be referring to this PR

My understanding from that discussion as well as this one (where I had previously advocated for it to be non-breaking) is that even in scenarios where consumers more than likely would not have used those exports, we were erring on the side of caution, and thus taking the more conservative approach to treat the removal of any exported code as breaking. I am open to discuss but this is my current understanding of the consensus around these types of changes.

[Torres-ssf]

I believe @arboleya is talking about #3020, specifically about this change here.

These operations names removed here were never used and implemented on the TS SDK, therefore aside from their names, they do not exist.

Incorrect

[petertonysmith94]

The OperationName being removed here, are being used in the fuels-wallet.

I believe we should just depreciate these.

[danielbate]

+1 with the sentiment here and exercising caution. Lets add to #3119.

[Torres-ssf]

The OperationName being removed here, are being used in the fuels-wallet.

I believe we should just depreciate these.

Good catch @petertonysmith94

@LuizAsFight I was under the impression that this was not being used.

[github-actions]

Coverage Report:

Lines	Branches	Functions	Statements
76.73%(+0.06%)	70.78%(-0.7%)	75.65%(+0%)	76.85%(+0.09%)

Changed Files:

Ok	File (✨=New File)	Lines	Branches	Functions	Statements
🔴	✨ internal/benchmarks/src/config.ts	0% (+0%)	0% (+0%)	0% (+0%)	0% (+0%)
🔴	internal/benchmarks/src/contract-interaction.bench.ts	0% (+0%)	0% (-100%)	0% (+0%)	0% (+0%)
🔴	internal/benchmarks/src/cost-estimation.bench.ts	0% (+0%)	0% (-100%)	0% (+0%)	0% (+0%)
🔴	internal/benchmarks/src/transaction-results.bench.ts	0% (+0%)	0% (-100%)	0% (+0%)	0% (+0%)
🔴	internal/benchmarks/src/wallet.bench.ts	0% (+0%)	0% (-100%)	0% (+0%)	0% (+0%)

[petertonysmith94]

@Torres-ssf Not sure why this one is failing

[Torres-ssf]

The OperationName being removed here, are being used in the fuels-wallet.

I believe we should just depreciate these.

@petertonysmith94 The enums are only being used within an image map, where the Wallet SDK assigns different images to operations based on their names. The idea is to display different images within the Wallet APP, based on the operation.

However, these operation enum entries were never actually used in assembling any operation. Operations are assembled entirely within the TS SDK side, and we never had code that used these enum entries.

Therefore, the Wallet SDK would simply need to remove the unused entries from the image map, when updating to the new fuels version, as this has no side effects whatsoever.

To address everyone's concerns that some users might be relying on these enums (even though it's highly unlikely, like a bald man using a comb 😆 ), I've marked each unused enum entry with the @deprecated tag.

@Torres-ssf Not sure why this one is failing

I am unsure why this one is falling. It seems to be failing on other PRs as well