Conversation
🦋 Changeset detectedLatest commit: e56aaba The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
abd0cfe to
7a45c2f
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces repo-wide support for authoring runtime modules in TypeScript using erasable type syntax, running .ts directly in tests via ts-blank-space/register, and ensuring published artifacts remain .js by erasing types and rewriting import specifiers during pack.
Changes:
- Add packing scripts to generate publishable
.jsfrom.ts(type erasure) and rewrite.tsimport specifiers to.js, plus an exports verifier. - Migrate initial packages (
@endo/eventual-send,@endo/patterns) to.tsruntime modules and update imports accordingly. - Update AVA/test and some runtime entrypoints (CLI/daemon/workers) to preload
ts-blank-space/register; standardizelint:eslinttoeslint ..
Reviewed changes
Copilot reviewed 73 out of 78 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds ts-blank-space (and its transitive TypeScript range) to the lockfile. |
| tsconfig.eslint-base.json | Allows .ts import specifiers for repo TS/ESLint configs. |
| scripts/packing/verify-package-exports.mjs | New script to validate package export specifiers by actually importing them. |
| scripts/packing/rewrite-ts-import-specifiers.mjs | New script to rewrite .ts import specifiers to .js in publish artifacts. |
| scripts/packing/prepack-package.mjs | New unified prepack pipeline: declarations, type-erased JS generation, delete .ts, rewrite specifiers. |
| scripts/packing/postpack-package.mjs | New unified postpack cleanup: restore sources/rewrites, clean generated artifacts. |
| scripts/packing/build-ts-to-js.mjs | New ts-blank-space-based .ts→.js transform for publish-time output. |
| packages/skel/package.json | Standardizes ESLint invocation to eslint .. |
| packages/patterns/types-index.js | Points runtime re-exports at patternMatchers.ts (rewritten during packing). |
| packages/patterns/types-index.d.ts | Updates comment to match .ts runtime source. |
| packages/patterns/tsconfig.json | Minor formatting change (trailing comma removed). |
| packages/patterns/tsconfig.build.json | Removes allowJs override and excludes src/exports.*. |
| packages/patterns/test/qp-on-pattern.test.js | Updates imports to patternMatchers.ts. |
| packages/patterns/test/patterns.test.js | Updates imports to patternMatchers.ts. |
| packages/patterns/test/pattern-limits.test.js | Updates imports to patternMatchers.ts. |
| packages/patterns/test/mismatch-stack-demo.test-verbose.js | Updates imports to patternMatchers.ts. |
| packages/patterns/test/mismatch-stack-demo.test-consise.js | Updates imports to patternMatchers.ts. |
| packages/patterns/test/copySet.test.js | Updates imports to patternMatchers.ts. |
| packages/patterns/test/copyMap.test.js | Updates imports to patternMatchers.ts. |
| packages/patterns/test/copyBag.test.js | Updates imports to patternMatchers.ts. |
| packages/patterns/test/containerHasSplit.test.js | Updates imports to patternMatchers.ts. |
| packages/patterns/test/_ava-stack-filtering-verbose.config.js | Preloads ts-blank-space/register for these AVA runs. |
| packages/patterns/test/_ava-stack-filtering-concise.config.js | Preloads ts-blank-space/register for these AVA runs. |
| packages/patterns/src/type-from-pattern.ts | Updates documentation references from .js to .ts. |
| packages/patterns/src/patterns/types.ts | Removes now-internalized helper types. |
| packages/patterns/src/patterns/patternMatchers.ts | Migrates runtime implementation to TS and internalizes helper types. |
| packages/patterns/src/patterns/getGuardPayloads.js | Updates import to patternMatchers.ts. |
| packages/patterns/src/keys/checkKey.js | Updates comment reference to patternMatchers.ts. |
| packages/patterns/package.json | Switches to unified prepack/postpack scripts. |
| packages/patterns/index.js | Re-exports from patternMatchers.ts. |
| packages/path-compare/package.json | Standardizes ESLint invocation to eslint .. |
| packages/panic/package.json | Standardizes ESLint invocation to eslint .. |
| packages/ocapn/package.json | Adds ts-blank-space dep and preloads it for a Node test command. |
| packages/ocapn-noise/package.json | Standardizes ESLint invocation to eslint .. |
| packages/marshal/test/_ava.config.mjs | New AVA config that preloads ts-blank-space/register. |
| packages/marshal/package.json | Moves AVA config into sesAvaConfigs and standardizes lint:eslint. |
| packages/lockdown/package.json | Standardizes ESLint invocation to eslint .. |
| packages/immutable-arraybuffer/package.json | Standardizes ESLint invocation to eslint .. |
| packages/harden/package.json | Standardizes ESLint invocation to eslint .. |
| packages/far/package.json | Standardizes ESLint invocation to eslint .. |
| packages/exo/package.json | Switches to unified prepack/postpack scripts. |
| packages/eventual-send/tsconfig.json | Enables allowImportingTsExtensions for this package. |
| packages/eventual-send/tsconfig.build.json | Removes allowJs override. |
| packages/eventual-send/test/_get-hp.js | Imports E.ts in tests. |
| packages/eventual-send/test/_ava.config.mjs | New AVA config that preloads ts-blank-space/register. |
| packages/eventual-send/src/types.d.ts | Re-exports types from E.ts (rewritten during packing). |
| packages/eventual-send/src/no-shim.js | Imports runtime E.ts. |
| packages/eventual-send/src/E.ts | New TS implementation of E. |
| packages/eventual-send/src/E.js | Removes old JS implementation of E. |
| packages/eventual-send/package.json | Uses AVA config with ts preload; switches to unified prepack/postpack; standardizes lint:eslint. |
| packages/evasive-transform/package.json | Standardizes ESLint invocation to eslint .. |
| packages/eslint-plugin/lib/configs/internal.js | Adds rule to disallow JSDoc type syntax in .ts. |
| packages/errors/package.json | Standardizes ESLint invocation to eslint .. |
| packages/daemon/test/_ava.config.mjs | New AVA config that preloads ts-blank-space/register. |
| packages/daemon/src/daemon-node-powers.js | Ensures child processes inherit --import ts-blank-space/register. |
| packages/daemon/package.json | Uses AVA config with ts preload; adds ts-blank-space dep. |
| packages/daemon/index.js | Ensures daemon subprocess inherits --import ts-blank-space/register. |
| packages/common/package.json | Standardizes ESLint invocation to eslint .. |
| packages/cli/package.json | Adds ts-blank-space dependency for runtime TS support. |
| packages/cli/bin/endo.cjs | Preloads ts-blank-space/register before running CLI entrypoint. |
| packages/captp/test/trap.test.js | Ensures Worker inherits --import ts-blank-space/register. |
| packages/captp/package.json | Standardizes ESLint invocation to eslint .. |
| packages/cache-map/package.json | Standardizes ESLint invocation to eslint .. |
| packages/bundle-source/test/_ava.config.mjs | New AVA config that preloads ts-blank-space/register. |
| packages/bundle-source/package.json | Uses AVA config with ts preload; standardizes lint:eslint. |
| packages/benchmark/rollup.config.js | Adds Rollup transform to erase types in .ts inputs. |
| packages/benchmark/package.json | Adds ts-blank-space dev dependency; standardizes lint:eslint. |
| package.json | Adds ts-blank-space dev dependency and root scripts for packing/verification. |
| docs/typescript.md | New contributor docs describing the erasable TS + packing workflow. |
| ava-noop-harden.config.mjs | Preloads ts-blank-space/register in this shared AVA config. |
| ava-endo-shims-only.config.mjs | Preloads ts-blank-space/register in this shared AVA config. |
| ava-endo-lockdown.config.mjs | Preloads ts-blank-space/register in this shared AVA config. |
| ava-endo-lockdown-unsafe.config.mjs | Preloads ts-blank-space/register in this shared AVA config. |
| ava-base.config.mjs | Preloads ts-blank-space/register in this shared AVA config. |
| .gitignore | Ignores .pack-rewrite-files.txt produced by packing scripts. |
| .changeset/ts-eslint-jsdoc.md | Changeset for eslint-plugin rule tweak. |
| .changeset/cli-ts-blank-space.md | Changeset for CLI/daemon runtime TS support. |
| .changeset/build-ts-support.md | Changeset for TS build infrastructure + initial migrations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| console.log(' → removing .ts source files from src/'); | ||
| try { | ||
| run('find', ['src', '-name', '*.ts', '!', '-name', '*.d.ts', '-delete']); | ||
| } catch { | ||
| // find may fail if src/ doesn't exist, which is fine | ||
| } |
There was a problem hiding this comment.
The prepack step deletes .ts files by shelling out to find. This is not portable (not available on Windows by default) and adds an extra external dependency beyond Node+git. Consider deleting *.ts (excluding *.d.ts) via a Node directory walk (similar to build-ts-to-js) so prepack works consistently across environments.
| // Determine package directory: explicit arg, INIT_CWD (set by yarn), or cwd | ||
| const packageDir = args.find(a => !a.startsWith('-')) || process.cwd(); | ||
| const srcDir = path.resolve(packageDir, 'src'); |
There was a problem hiding this comment.
The comment says the package directory is determined from an explicit arg, INIT_CWD, or cwd, but the implementation only checks args and process.cwd(). Either include process.env.INIT_CWD in the logic (to match other packing scripts) or update the comment so it matches the actual behavior.
829fbb4 to
8e7a988
Compare
|
Somewhat related - getting compartment-mapper to run typescript is not a lot of work LavaMoat/LavaMoat#1894 |
I'm confused, I thought the community recommendation (based on Node.js behavior) is still to not publish |
mhofman
left a comment
There was a problem hiding this comment.
I only reviewed the docs and package.json.
We're lacking instructions (and likely lint rules) enforcing that only imports from non node_modules can use .ts extensions to satisfy Node.js semantics. While cross-package .ts imports will work within the same repo, I am concerned about blurring the lines in authoring rules (and complexity of enforcement) if we rely on same repo cross package imports to be rewritten.
Also since we're now allowing and encouraging .ts source, these files should be included in the package so that "Navigate to source" continues working (the .d.ts.map would point at the .ts source file)
| - **At development time** — imported directly as `.ts` (via a type-stripping | ||
| loader or Node's built-in `--experimental-strip-types`) |
There was a problem hiding this comment.
Make it clear this is only for same package (or same repo) relative imports.
|
|
||
| ### Import specifiers | ||
|
|
||
| In `.ts` source files, import other `.ts` files using `.ts` specifiers: |
There was a problem hiding this comment.
Clarify that for runtime imports, .ts should only be used for relative (same package / same repo) imports.
There was a problem hiding this comment.
We need to include some guidance for package entrypoints authored in .ts. Since Node.js will not allow importing non-relative .ts specifiers (well more precisely .ts from under node_modules, a package entrypoint should be a .js file in source. However that .js file can simply contain export * from './src/index.ts' (or wherever the ts implementation is, as long as it's not the same file name).
Alternatively, if we want to use .ts entrypoints, we must have linting rules that only allow cross-package .ts if the target is in the same repo, and that rewrites processes these and only these (not arbitrary packages under node_modules)
| export type * from './src/types.js'; | ||
| ``` | ||
|
|
||
| This pattern is being replaced by direct `.ts` exports as packages migrate. |
There was a problem hiding this comment.
"direct .ts exports" is a bit misleading, when .ts files cannot be entrypoints.
boneskull
left a comment
There was a problem hiding this comment.
From my limited experience with type-stripping, I am loathe to accept the complexity. This complexity is further compounded by our constraints and historical choices.
There's too much "bespoke build system" here. As much as I'd love to write TS instead of JS, I'm unhappy with this direction.
I'm not going to block, because I assume I'm in the minority here.
I hope to be pleasantly surprised, but for now I will grit my teeth.
| @@ -1,4 +1,5 @@ | |||
| // @ts-check | |||
| /* global process */ | |||
There was a problem hiding this comment.
Would it be prudent to actually configure ESLint for the node env?
There was a problem hiding this comment.
Eslint has deprecated their eslint-env directive. We could configure by file path overrides but we don't have a convention for what modules will run (only) in Node or not.
| @@ -0,0 +1,6 @@ | |||
| export default { | |||
There was a problem hiding this comment.
Are we moving AVA configs into standalone files from package.json?
There was a problem hiding this comment.
Not generally. This was just so the Ava config could have a comment. Darn package.json!
|
|
||
| let hasChanges = false; | ||
|
|
||
| for (const tsPath of tsFiles) { |
There was a problem hiding this comment.
What prevents this from running in parallel?
| console.error( | ||
| '\nRun "scripts/packing/build-ts-to-js.mjs" to update generated files', | ||
| ); | ||
| process.exit(1); |
There was a problem hiding this comment.
| process.exit(1); | |
| process.exitCode = 1; | |
| return; |
|
|
||
| main().catch(err => { | ||
| console.error(err); | ||
| process.exit(1); |
There was a problem hiding this comment.
| process.exit(1); | |
| process.exitCode = 1; |
generally you only want to use process.exit() with sync code.
| @@ -0,0 +1,118 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
I really don't like all of the complexity involved in this build process. It's going to suck if something breaks in the middle of it.
Off the top of my head, given our constraints (grr), is that maybe we should do all of this work in a transient directory somewhere.
- Generate
.d.tsfiles; move/copy them into a transient dir structure - Suck in all the files we need for publish (
package.json,README.md,LICENSE, whatever's inpackage.json'sfiles), strip types & rename extensions as necessary, and poop out the result into the dir structure alongside the declarations. A Gulp-esque pipeline - Pack / publish from the transient dir.
Then we shouldn't need to cleanup anything.
Rant:
We should be using incremental TS builds. I've always been grouchy about the need to remove
.d.tsfiles. Will stop harping on this for now.
There was a problem hiding this comment.
I agree. Last week I made a ts-node-pack CLI that does that and would make this PR much simpler. My kid was sick and is on vacation this week so I had to pause it but I'll try to get that out and redo this using that.
It would be a trusted dep in the build system but a very light one easy to audit.
There was a problem hiding this comment.
If building the output in a temp dir works I'm ok with that. I remember some issue with tsc outDir when generating d.ts, but maybe I'm confusing things.
There was a problem hiding this comment.
It took more work than I expected but it seems to work.
| '@endo/exo': patch | ||
| --- | ||
|
|
||
| Add build infrastructure for `.ts` source modules using erasable type syntax |
There was a problem hiding this comment.
I don't understand why we need ts-blank-space and can't use Node.js' facilities.
There was a problem hiding this comment.
strong +1 to that. an exact copy of the functionality ships with node.js now and is available as a function (see my link in the first comment for inspiration)
There was a problem hiding this comment.
Older versions of node don't support type stripping. I believe this is there until the minimum supported node version does.
There was a problem hiding this comment.
Node also only strips on loading. It doesn't have a way to strip as a transform.
It's app only. I limit type stripping to the sources in root compartment. |
rely on eslintignore for what to ignore. this aligns with root `eslint .`
Yarn isn't a publishing tool and has some lax semantics
The refactor to ts-node-pack (which enumerates files via npm-packlist)
stopped shipping the extensionless 'bin/<name>' symlinks that our
previous 'yarn pack' flow included. Three packages had such symlinks:
- @endo/ses-ava bin/ses-ava -> ses-ava.cjs
- @endo/cli bin/endo -> endo.cjs
- @endo/bundle-source bin/bundle-source -> bundle-source.cjs
(orphaned; its 'bin' field already points elsewhere)
Of those, 'ses-ava' and 'cli' had package.json 'bin' entries that
pointed at the symlink. Post-refactor the tarball had no file at that
path, so 'npm publish --dry-run' warned 'No bin file found at
bin/ses-ava' and the installed CLI would be broken.
Why yarn pack handled this and npm-packlist does not:
yarn pack walks the file tree itself and writes symlinks into the
tarball as POSIX tar symlink entries (type flag '2'), which extract
back into real symlinks on install. npm-packlist -- the enumeration
library shared by npm pack, yarn npm publish, and ts-node-pack --
filters symlinks out entirely. This is a deliberate, long-standing
choice in the npm CLI: symlinks in published tarballs are considered
a reproducibility and security hazard (a symlink to an absolute path
or an upward relative path can escape the install prefix), so the
packer refuses to emit them regardless of whether the tarball format
permits it. The consequence is that any package relying on a symlink
to satisfy its 'bin' field is silently broken under npm pack -- as
@endo/ses-ava@1.4.0 already is on the registry today, which is how
this survived unnoticed: the published tarball has no bin/ directory
at all, so nobody's ever been able to run the CLI from an install.
Fix: point the 'bin' fields at the real .cjs files and delete the now
unreferenced symlinks. The installed command name is unchanged -- for a
scoped package with a string 'bin', npm derives the command name from
the package name with the scope stripped (@endo/ses-ava -> ses-ava),
not from the target file name; for @endo/cli's object form, the key
'endo' remains the command name. After this change 'npm pack',
'yarn pack', and 'ts-node-pack' all produce equivalent, working
tarballs.
Why drop the extensionless form entirely rather than preserve it:
Three alternatives were considered and rejected.
(1) Keep the symlinks in the source tree but point 'bin' at the
.cjs files. This fixes the publish path (the tarball no longer
references a missing file) but leaves the symlinks as dead
weight that any future contributor will puzzle over: an
extensionless name that looks like an entry point but is
referenced by nothing in package.json. The name would also
remain a cross-platform hazard (see below), and would continue
to drop out of every npm pack locally -- which is how the
original ses-ava@1.4.0 breakage slipped through for years.
(2) Replace each symlink with a real one-line shim file -- e.g.
bin/bundle-source containing '#!/usr/bin/env node' plus
require('./bundle-source.cjs'). This survives npm-packlist, so
the extensionless name keeps working locally and inside
tarballs. But the shim is a second entry point that has to be
kept in sync with the real one, npm-packlist will ship it into
every consumer install as dead bytes, and it invites the next
contributor to wonder which of the two files is authoritative.
Two entry points where one suffices, indefinitely.
(3) Leave the 'bin' field pointing at the symlink and hope nobody
notices. This is the status quo that produced the broken
ses-ava@1.4.0 tarball on the real registry. No.
The extensionless 'bin/<name>' form only ever worked on POSIX
filesystems anyway: git on Windows does not materialize symlinks
without core.symlinks=true, which most contributors never set, so
Windows devs checking out this repo got a plain text file
containing 'ses-ava.cjs' (the link target as literal text) where
bin/ses-ava was supposed to be. The form we are dropping was never
portable; removing it makes the repo match the tarballs that ship
and makes the entry points work consistently across platforms and
packers. The only behavioural change a human might notice is that
'cd packages/ses-ava && ./bin/ses-ava' at the shell no longer tab-
completes to a runnable path -- the replacement is to run
'./bin/ses-ava.cjs' or just the installed 'ses-ava' command name,
both of which already worked and keep working.
Test callers and one demo alias that referenced the extensionless
form are updated in the same commit so nothing depends on the
removed names.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Convert src/E.js to TypeScript source using erasable type syntax. Update import specifiers in no-shim.js, types.d.ts, and test/_get-hp.js to use .ts extensions. Enable allowImportingTsExtensions in tsconfigs. Use unified prepack/postpack scripts for publishing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces the in-place prepack/postpack mutation pipeline (scripts/packing
plus per-package lifecycle hooks) with a tarball-native flow:
- scripts/pack-all.mjs builds every public workspace into dist/, using
ts-node-pack for packages with .ts sources and 'yarn pack' for the
pure JS+JSDoc majority.
- scripts/release-npm.mjs replaces 'lerna publish from-package': it
re-runs pack-all then 'npm publish' each tarball, honoring --tag.
- scripts/files.sh enumerates from dist/ instead of running pack
per-package.
Removes prepack/postpack hooks from exo, patterns, and eventual-send,
deletes scripts/packing/, and drops the .pack-* manifest infrastructure
\u2014 the source tree is no longer mutated during packing.
Adds ts-node-pack as a portal devDep, plus two upstream fixes:
- findLocalBin falls back to 'yarn bin tsc' for Yarn 4 pnpm/PnP linkers.
- typeRoots resolution walks up to find @types when the per-package
node_modules is empty (Yarn 4 pnpm linker layout).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds scripts/smoketest-publishing.sh, which boots a disposable Verdaccio
registry via 'npx verdaccio', runs 'yarn release:npm' against it (so the
full pack-all + npm-publish loop is exercised end-to-end), then installs
a representative subset of the published tarballs into a fresh consumer
and imports them under SES lockdown. The pattern mirrors agoric-sdk's
scripts/packing/smoketest-publishing.sh but is much smaller: endo
publishes already-built tarballs from dist/ rather than running lerna
publish against the source tree, so there is no dev-version bump, no
lerna, and no git state dance.
Key design points:
- A custom verdaccio.yaml disables the default npmjs.org uplink
('uplinks: {}'). Without this, Verdaccio proxies reads upstream and
refuses to let us publish over versions that already exist on the
public registry, since endo's tarballs carry real release numbers
rather than dev prereleases.
- A free TCP port is picked via Node's net.createServer() rather than
hardcoding 4873, so a stray Verdaccio on the conventional port can't
collide with us and we can't collide with it.
- HOME is a per-run mktemp dir, so the script never touches the
developer's ~/.npmrc or leaves auth tokens behind. A SIGTERM/SIGINT
trap cleans up Verdaccio and the dir on exit.
- 'npm_config_registry' is inlined on the yarn release:npm command
only, not exported globally. Exporting it causes 'npx npm-cli-login'
earlier in the script to try to fetch npm-cli-login itself from the
empty local registry and 404.
- The consumer install uses 'npm install <name>' (not '<path>') so
npm has to resolve each package from the registry. That proves the
rewritten 'workspace:' deps resolved to concrete versions that
actually exist in the same registry, and that the cross-package
type graph is self-consistent in tarball form.
Wired up as 'yarn smoketest:publish' and invoked from ci.yml's build
matrix, replacing the prior pair of steps:
- 'yarn workspaces foreach --all --topological exec npm pack', which
could not have worked after the ts-node-pack refactor: the .ts-using
packages (exo, patterns, eventual-send) no longer have prepack
hooks, so npm pack would have shipped raw .ts files.
- 'yarn lerna run prepack' for cross-package type-resolution
validation. That check is now subsumed by the consumer import step:
if a package's declarations depend on a neighbor whose published
form is missing or mis-resolved, the import will fail under SES.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 87 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const repoRoot = path.resolve(new URL('..', import.meta.url).pathname); | ||
| const distDir = path.join(repoRoot, 'dist'); |
There was a problem hiding this comment.
repoRoot is derived via new URL(..).pathname, which is not reliably portable (e.g., URL-encoding and Windows drive-letter paths). Prefer fileURLToPath(new URL('..', import.meta.url)) (then path.resolve(...) if needed) to get a correct filesystem path across platforms.
| # Build every workspace's tarball into dist/ via pack-all (which uses | ||
| # ts-node-pack for .ts packages and yarn pack for the rest). | ||
| yarn pack:all 1>&2 |
There was a problem hiding this comment.
The comment says pack-all uses “ts-node-pack for .ts packages and yarn pack for the rest”, but this script always invokes ts-node-pack for every workspace. Please update the comment to match the actual behavior (ts-node-pack can still emit verbatim JS tarballs for non-TS packages).
| for TGZ in "$DIST"/*.tgz; do | ||
| PKG_NAME="$(basename "$TGZ" .tgz)" | ||
| # shellcheck disable=SC2016 | ||
| PKG="$PKG_NAME" "$TAR" xf "$TGZ" --to-command='echo "$PKG/${TAR_FILENAME#package/}"' | ||
| done | sort |
There was a problem hiding this comment.
for TGZ in "$DIST"/*.tgz will iterate once with the literal pattern if there are no matches (nullglob is off by default), causing a confusing tar failure. Consider enabling nullglob for this loop or explicitly checking that at least one .tgz exists in dist/ before iterating.
| echo "smoketest-publishing: starting Verdaccio (HOME=$REGISTRY_HOME)" | ||
| ( | ||
| cd "$REGISTRY_HOME" | ||
| : > verdaccio.log | ||
| nohup npx --yes verdaccio@^6 --config "$REGISTRY_HOME/verdaccio.yaml" \ | ||
| --listen "$REGISTRY_PORT" &> verdaccio.log & | ||
| echo $! > verdaccio.pid | ||
| # Block until verdaccio prints its "http address" line to the log. | ||
| grep -q 'http address' <(tail -f verdaccio.log) | ||
| ) |
There was a problem hiding this comment.
The Verdaccio “wait until ready” step (grep -q ... <(tail -f ...)) can hang indefinitely if Verdaccio fails to start or the expected log line never appears. Add a bounded timeout and/or a readiness probe (e.g., retry npm ping --registry $REGISTRY_URL / curl until success) so CI fails fast instead of stalling.
| "release:npm": "scripts/release-npm.mjs", | ||
| "pack:all": "scripts/pack-all.mjs", | ||
| "smoketest:publish": "scripts/smoketest-publishing.sh", |
There was a problem hiding this comment.
The repo declares engines.node: >=16, but this PR introduces multiple uses of Node’s --import ts-blank-space/register preload mechanism (AVA configs, workers, daemon child processes, etc.), which is not supported on all Node versions >=16. Please align the declared Node engine(s) with the minimum version required by the new runtime/test bootstrapping, or switch to a preload mechanism that works on the supported range.
| // The runtime implementations live in patternMatchers.ts (which uses | ||
| // @ts-nocheck and cannot be directly type-checked) and are re-exported | ||
| // by types-index.js. These declarations overlay the runtime types so | ||
| // that consumers get type-narrowing for free. |
There was a problem hiding this comment.
This comment says patternMatchers.ts “uses @ts-nocheck and cannot be directly type-checked”, but patternMatchers.ts no longer appears to include @ts-nocheck (it has explicit TypeScript types now). Please update the comment to reflect the current type-checking approach (or add @ts-nocheck if that is still intended).
| const tagFlag = process.argv.indexOf('--tag'); | ||
| const tag = tagFlag >= 0 ? process.argv[tagFlag + 1] : process.env.npm_config_tag; | ||
|
|
There was a problem hiding this comment.
The header comment says this script honors both NPM_CONFIG_TAG and npm_config_tag, but the implementation only reads process.env.npm_config_tag. Either read process.env.NPM_CONFIG_TAG as well (with a clear precedence order) or adjust the comment so it matches the behavior.
Refs: #2864
Description
Adds build infrastructure and initial migrations for writing Endo package source in TypeScript using erasable type syntax (types that can be removed without changing runtime semantics). Published packages continue to ship
.js— types are erased at build/pack time viats-blank-space.Key changes:
scripts/packing/):prepack-package/postpack-packagescripts that erase.ts→.jsfor publishing and rewrite import specifiersts-blank-space/registerso.tssource runs directly in tests@endo/eventual-send: MigrateE.js→E.ts@endo/patterns: MigratepatternMatchers.js→patternMatchers.ts, encapsulating helper types that were in a separatetypes.ts@endo/eslint-plugin: Catches JSDoc type annotations in.tsfiles@endo/cli/@endo/daemon: Loadts-blank-spacefor Node.js.tsmodule supporteslint .(relying on.eslintignore) instead of explicit glob patternsdocs/typescript.mddocuments the approachSecurity Considerations
No new authorities or trust boundaries. Type erasure is a build-time transformation; runtime semantics are unchanged.
Scaling Considerations
No significant resource changes.
ts-blank-spaceis a fast single-pass transform.Documentation Considerations
docs/typescript.mdis added to guide contributors on writing.tsmodules in this repo.Testing Considerations
Existing test suites pass against the migrated
.tssource viats-blank-space/register. A test is added for Node < 22 compatibility withts-blank-space.Compatibility Considerations
Published packages continue to ship
.jsfiles with the same exports. No breaking changes to downstream consumers.Upgrade Considerations
None — published artifacts are unchanged.