Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 207 forks source link

fix(ertp): remove unneeded ertp type imports #10467

Closed erights closed 1 week ago

erights commented 1 week ago

closes: #XXXX refs: #XXXX

Description

For some reason, vscode does successfully highlight unused jsdoc type imports

image

but yarn lint does not

image

This PR started as a drive-by extracted from https://github.com/Agoric/agoric-sdk/pull/10456 , where I noticed that ERTP in particular had some of these. Since #10456 experiments with a variation on ERTP, simplifying it first in harmless ways helps a bit.

Security Considerations

fewer distractions helps

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

none

Upgrade Considerations

In the agoric-sdk repo there are no remaining type imports of the legacy type reexport of Baggage from ertp. But if there are such imports in other repos, they will need to be fixed. But this is a static-only issue, not a runtime issue, so it's fine for those repos to be fixed only once they depend on the new @agoric/ertp without that legacy reexport.

cloudflare-workers-and-pages[bot] commented 1 week ago

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: fe06d31
Status: ✅  Deploy successful!
Preview URL: https://d1490a6a.agoric-sdk.pages.dev
Branch Preview URL: https://markm-remove-unneeded-ertp-t.agoric-sdk.pages.dev

View logs

erights commented 1 week ago

What's the harm in leaving around extra type imports?

Distraction. Makes it a bit harder to find the type imports that are actually used. Also, makes it a bit harder to search for "who meaningfully imports this type?".

Same rationale for avoiding unused imports and other dead notation in general, though clearly of weaker magnitude for type imports.