aptos-core
aptos-core copied to clipboard
[Bug] make `instanceof` more robust in `aptos` sdk
🐛 Bug
Use aptos version 1.3.15 to construct a transaction, and send it using aptos version 1.3.16, this exception will be triggered.
In complex projects it is common to see multiple versions of aptos sdk present as the project may depend on lots of other packages that use their own aptos versions. We need multiple versions of aptos sdk to maintain some level of compatibility. The use of the instanceof keyword is breaking that compatibility. This problem could be remedies by using a mechanism similar to Symbol.hasInstance
To reproduce
Use aptos version 1.3.15 to construct a transaction, and send it using aptos version 1.3.16, this exception will be triggered.
Thanks for reporting! It is dangerous to pass instances across different versions of SDKs. In my opinion, JS did the right thing by not passing the instanceof check. Imagine that, we added a field or deleted a field to the same class in the new version of SDK, passing instances from the old SDK would cause unknown issues if instanceof passed. Instead, I would assume class instances are different across package boundaries. Thoughts?
@Manahip If you'd be willing to suggest a fix, have us agree on it, and then implement it, that'd be much appreciated. Given that this is a somewhat niche problem, we might not have time to address this in the near future.
This issue is stale because it has been open 45 days with no activity. Remove the stale label or comment - otherwise this will be closed in 15 days.
Thanks for reporting! It is dangerous to pass instances across different versions of SDKs. In my opinion, JS did the right thing by not passing the
instanceofcheck. Imagine that, we added a field or deleted a field to the same class in the new version of SDK, passing instances from the old SDK would cause unknown issues ifinstanceofpassed. Instead, I would assume class instances are different across package boundaries. Thoughts?
actually... the typescript team itself avoids the instanceof keyword like plague. When a field is deleted from any class, it is ground for major version bump. It really shouldn't break across minor or bug fix versions.
@Manahip If you'd be willing to suggest a fix, have us agree on it, and then implement it, that'd be much appreciated. Given that this is a somewhat niche problem, we might not have time to address this in the near future.
This is an example, using an explicit kind tag: https://github.com/microsoft/TypeScript/blob/main/lib/typescript.d.ts#L847
also, now that we're past mainnet launch.. probably it's time to pick up semantic versioning and bump the sdk's major version to 100 or something :P