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.

Sign up for GitHub

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

fix(ses): fix types export for newer module resolutions #1840

Merged
merged 2 commits into from
Nov 6, 2023
Merged

fix(ses): fix types export for newer module resolutions #1840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cc3dd5
fix(ses): fix types export for newer module resolutions
boneskull Oct 27, 2023
fe38c40
fix(ses): fix ThirdPartyStaticModuleInterface type
boneskull Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/ses/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@
"exports": {
".": {
"import": "./index.js",
"require": "./dist/ses.cjs"
"require": "./dist/ses.cjs",
"types": "./types.d.ts"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a line to package.json.md explaining this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kriskowal Done. Also fixed another path in there that was wrong.

},
"./lockdown": {
"import": "./index.js",
"require": "./dist/ses.cjs"
"require": "./dist/ses.cjs",
"types": "./types.d.ts"
},
"./tools.js": "./tools.js",
"./package.json": "./package.json"
Expand Down
24 changes: 16 additions & 8 deletions packages/ses/package.json.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This is an explainer for the module system configuration of the SES shim
package through some properties of `package.json`.

## "type": "module",
## "type": "module"

For Node.js, this means that any `.js` file will be interpreted as if it were
`.mjs`, meaning a JavaScript module / ESM, as opposed to `.cjs` which is
Expand All @@ -14,7 +14,7 @@ and translates all `.js` files down from ESM to CommonJS.
A patch to `esm` that is in the Agoric SDK repository allows `.js`
files in dependencies to break through to the underlying Node.js ESM.

## "main": "./dist/ses.cjs",
## "main": "./dist/ses.cjs"

SES provides its own translation from its own ESM sources to CommonJS, emitted
by `yarn build`, specifically `scripts/bundle.js`.
Expand All @@ -35,7 +35,7 @@ The `main` property has been supported by the npm ecosystem since the
earliest versions, so every version of Node.js and every tool will look
here if nothing else in `package.json` overrides it.

## "module": "./index.js",
## "module": "./index.js"

Some tools like WebPack, Parcel and the ESM emulation provided by `node -r esm`
use this instead of `main` if it is present.
Expand All @@ -49,7 +49,7 @@ non-ASCII characters (we use zero-width-joiner to avoid collisions with other
names in scope, then censor the use of zero-width-joiner in source).
Most tools tolerate this, but WebPack does not.

## "unpkg": "./dist/ses.umd.js",
## "unpkg": "./dist/ses.umd.js"

The [Unpkg][] CDN uses this property to direct usage of SES to a precompiled
module in "Universal Module Definition" format.
Expand All @@ -60,7 +60,7 @@ a `<script>` tag.

[Unpkg]: https://unpkg.com/

## "types": "./index.d.ts",
## "types": "./types.d.ts"

TypeScript uses this to transport type definitions.
These include type declarations (which are scoped like module exports)
Expand Down Expand Up @@ -102,11 +102,17 @@ We have in the past experimented with using the precompiled bundle of SES here
(`./dist/ses.cjs` or `./dist/ses.umd.js`), but found that this interacted
poorly with Endo, because an Endo bundle contains identifiers that SES censors.

## "require": "./dist/ses.cjs",
## "require": "./dist/ses.cjs"

Node.js and other tools will use this file when importing `ses` as an CommonJS module.

## "./lockdown": ...
## "types": "./types.d.ts"

Only applicable for TypeScript v4.7+ consumers configured with `node16` or
`nodenext` [module resolution][]. This serves the same purpose as the `types`
prop at the top level.

## "./lockdown"

The most recent SES only provides one API, but a previous version
exported a separate `ses/lockdown` layer.
Expand All @@ -115,7 +121,7 @@ is deprecated.

The value is the same as for `"."` above.

## "./tools.js": ...
## "./tools.js"

Exposes tools that are useful for creating hardened JavaScript environments that move certain responsibilities from `eval` to generated code.

Expand All @@ -125,3 +131,5 @@ Tools like Svelte need to access the `package.json` of every package in an
application.
Node.js versions with JavaScript module support require this "module" to be
made expressly public.

[module resolution]: https://www.typescriptlang.org/docs/handbook/modules/theory.html#module-resolution
5 changes: 4 additions & 1 deletion packages/ses/types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,11 @@ export interface PrecompiledStaticModuleInterface {
export interface ThirdPartyStaticModuleInterface {
imports: Array<string>;
exports: Array<string>;
/**
* Note that this value does _not_ contain any numeric or symbol property keys, which can theoretically be members of `exports` in a CommonJS module.
*/
execute(
proxiedExports: Object,
proxiedExports: Record<string, any>,
compartment: Compartment,
resolvedImports: Record<string, string>,
): void;
Expand Down