Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate weird redundant globals.d.ts files #4830

Closed
FUDCo opened this issue Mar 11, 2022 · 6 comments
Closed

Eliminate weird redundant globals.d.ts files #4830

FUDCo opened this issue Mar 11, 2022 · 6 comments
Assignees
Labels
tooling repo-wide infrastructure
Milestone

Comments

@FUDCo
Copy link
Contributor

FUDCo commented Mar 11, 2022

In the course of maintaining the virtual objects stuff, whenever a change to the VatData API happens (which has been happening quite a bit as it's under active development and evolving), it is necessary to perform coordinated changes to 6 separate globals.d.ts files in 6 separate packages:

packages/xsnap/src/globals.d.ts
packages/deploy-script-support/globals.d.ts
packages/run-protocol/globals.d.ts
packages/ERTP/globals.d.ts
packages/pegasus/globals.d.ts
packages/vats/globals.d.t

With the exception of the xsnap package, all of these files are identical and only declare VatData and its related parts, and even xsnaps's does little more than that.

This is ludicrous, annoying, and potentially dangerous.

If we can't get rid of these silly things, can we at least have just one of them in some standard place that everybody else can point to?

@FUDCo FUDCo added the bug Something isn't working label Mar 11, 2022
@FUDCo
Copy link
Contributor Author

FUDCo commented Mar 11, 2022

@erights

@FUDCo
Copy link
Contributor Author

FUDCo commented Mar 11, 2022

Tagging @turadg @mhofman @michaelfig and please also add any of our other TypeScript enthusiasts I might have missed

@mhofman
Copy link
Member

mhofman commented Mar 11, 2022

The solution is, you guessed it, move away from ambient types. A module file (containing an import or export) can declare global. Then you should be able to have one declaration and import the rest from there? Also from a cursory look, all these files are all pretty much incorrect and basically won't do what you want besides typing the makeFoo and any.

@FUDCo
Copy link
Contributor Author

FUDCo commented Mar 11, 2022

I literally don't care what the solution is. I just want somebody to make this go away.

@mhofman
Copy link
Member

mhofman commented Mar 11, 2022

I think this could and should be tackled at the same time as #4560, but there may be a quicker fix.

@turadg
Copy link
Member

turadg commented Mar 11, 2022

For @FUDCo 's particular grief with updating VatData we could import the def from a canonical place. #4255

I'll assign myself.

@turadg turadg self-assigned this Mar 11, 2022
@turadg turadg added tooling repo-wide infrastructure and removed bug Something isn't working labels Mar 11, 2022
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@turadg turadg closed this as completed Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling repo-wide infrastructure
Projects
None yet
Development

No branches or pull requests

4 participants