restore .d.ts emit for several packages #9189
Conversation
This reverts commit dbb6522.
|
|
||
| // XXX re-export types into global namespace, for consumers that expect these to | ||
| // be ambient. Why the _ prefix? Because without it TS gets confused between the | ||
| // import and export symbols. h/t https://stackoverflow.com/a/66588974 |
There was a problem hiding this comment.
devious...
This might be a hack as this raises an error if placed inside a .ts file but seems to work when placed inside a .d.ts file.
| // XXX re-export types into global namespace, for consumers that expect these to | ||
| // be ambient. Why the _ prefix? Because without it TS gets confused between the | ||
| // import and export symbols. h/t https://stackoverflow.com/a/66588974 | ||
| // Note one big downside vs ambients is that these types will appear to be on `globalThis`. |
There was a problem hiding this comment.
fair price, given that this is a transitional measure.
Is it worth an UNTIL comment?
packages/governance/src/index.js
Outdated
| // XXX ambient types runtime imports | ||
| import '@agoric/ertp/exported.js'; | ||
| import '@agoric/zoe/src/contractFacet/types-ambient.js'; | ||
| import '@agoric/zoe/tools/types-ambient.js'; | ||
| import '@agoric/zoe/src/types.js'; |
There was a problem hiding this comment.
That's an unfortunate regression... but I see those imports are also in agoric-upgrade-13 ... well, a similar list.
fwiw, as of agoric-upgrade-13, the comment at the top cited #6512 ; might be nice to put that back
packages/governance/tsconfig.json
Outdated
| "exported.js", | ||
| // omit exported.js because 1) it's empty and would overwrite exported.d.ts | ||
| // and 2) because it's only for consumption in other packages |
There was a problem hiding this comment.
I'm a little confused.
"for consumption in other packages" seems like a reason to include it, not a reason to omit it.
Does exported.d.ts suffice somehow when other packages want exported.js?
There was a problem hiding this comment.
that it's used in other packages is a reason to include it in package.json files. but this is the tsconfig for typechecking this package, so it's superfluous. and because there's a .d.ts it needn't be part of the build. I'll clarify
turadg
force-pushed
the
ta/fix-zoe-prepack
branch
from
b4d6f94
to
9372618
Compare
no ticket
Description
@dckc discovered that the Zoe package in NPM no longer had
.d.tsfiles. We bisected it to dbb6522The
--cleanmade the build command succeed, but stopped actually emitting the files. The motivation for the--cleanwas to work around the use of/// <reference pathin which some packages were reaching into other packages through the filesystem.The undoes dbb6522 and solves the type resolution problem using the new
@importinstead of the reference directive. That in turn required making@agoric/governanceexport explicit types, the way@agoric/ertpwas recently made to do.Security Considerations
n/a
Scaling Considerations
n/a
Documentation Considerations
Nothing for end users
Testing Considerations
Upgrade Considerations
does not affect vats