Skip to content

loader: standalone tsx-style npm loader package - #810

Merged
colinhacks merged 10 commits into
mainfrom
loader
Sep 1, 2026
Merged

loader: standalone tsx-style npm loader package#810
colinhacks merged 10 commits into
mainfrom
loader

Conversation

@colinhacks

@colinhacks colinhacks commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Ships the runtime's resolve + transpile hooks as a standalone npm package: node --import @nubjs/loader, --require @nubjs/loader, @nubjs/loader/esm. Slim entries under runtime/loader-* arm only the shared transform-core / preload-common hooks; the addon rides 8 @nubjs/loader-<platform> packages resolved at runtime (__NUB_ADDON_PATH, probed last). Version/lockstep/release wiring, a docs page, and a tarball-install matrix under tests/loader/ are included.

Name: @nubjs/loader (bare nubjs is refused by npm's similarity guard; scoped names bypass it). Before the first release that ships it, run scripts/npm-first-publish-loader.sh once — npm OIDC cannot create a package (npm/cli#8544), so the nine new packages must exist and be trusted first.

Verified from a clean tarball install: --import green on Node 18.19–26.7; --require on 20.19+/22.12+.

…n plumbing

Adds runtime/loader-*.{mjs,cjs}: slim entrypoints that arm only the shared
resolve+transpile hooks (transform-core + preload-common) for
node --import <pkg> / --require <pkg> / <pkg>/esm, with the addon resolved from
a per-platform npm package via __NUB_ADDON_PATH (probed last by transform-core).
Adds npm/loader + npm/loader-<platform> package manifests, the staging script,
and version/lockstep/gitignore plumbing.
release.yml stages a loader-<platform> addon artifact per build leg and
publishes the 8 addon packages then the root loader package (stable + canary),
mirroring the CLI's ordering. Adds site/content/docs/loader.mdx and a
tarball-install matrix under tests/loader/ (--import on every tier, --require
where require(esm) exists, optional tsx differential).
Copilot AI lite review requested due to automatic review settings August 28, 2026 23:07
@pullfrog

pullfrog Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 12:40am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nub Ready Ready Preview Aug 31, 2026 10:54pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@pullfrog

pullfrog Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 12:40am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

npm's OIDC trusted publishing cannot create a package (npm/cli#8544), so the
nine new loader packages must exist and be trusted before the release that
ships them. Maintainer-run, idempotent.
npm's similarity guard refuses new unscoped names near existing ones (nubjs →
403 vs fbjs/runjs); scoped names bypass the guard and the org owns the scope.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

One correctness defect in the compat-tier arming path, plus two release-process risks worth a decision before the first tag.

Reviewed changes

  • Traced the arm() tier decision in runtime/loader-entry.mjs across (Node version × --import/--require × foreign loader × worker thread), against runtime/preload.cjs's fast-tier reference.
  • Verified the RUNTIME_FILES transitive closure in scripts/build-loader-npm.mjs — every relative import of every staged file is present, and runtime/version.mjs genuinely is not needed.
  • Checked the release wiring end to end: per-target addon staging, VERSION availability in the two new publish steps, and version lockstep at 0.8.0 across npm/nub, npm/loader, and all eight npm/loader-* manifests.
  • Confirmed the npm trust CLI contract and the npm/cli#8544 first-publish premise behind scripts/npm-first-publish-loader.sh.

Cleared on inspection, recording so they don't get re-raised: the root .gitignore cannot strip staged files from the tarball (npm's files[] allowlist wins); armed really is shared across a combined --import + --require delivery, because require(esm) and import() of the same resolved .mjs share one module record; the unscoped nubjs name is a deliberate carve-out and is documented as one in the wiki/agents.md diff.

⚠️ The release can leave npm half-published

The loader publish step is appended to the end of publish-npm (.github/workflows/release.yml), after @nubjs/nub and its platform packages have already published irreversibly. Nothing in the workflow gates the release on scripts/npm-first-publish-loader.sh having been run.

If it hasn't been (or its trust step failed quietly — see the inline comment on it), the step fails on the first loader package, and the tag lands with @nubjs/nub@X on npm and no matching nubjs@X. test-install blocks on publish-npm, so it won't catch it either.

Recovery is real — every step is idempotent, so running the one-time script and re-running the job fixes it. But the window where the two package families disagree is user-visible, and the ordering makes the loader the thing that fails last rather than first. Two options, both cheap: move a "loader packages exist and are trusted" precondition check into the verify job so a missing setup fails before anything publishes, or move the loader publish ahead of @nubjs/nub so the irreversible step is last.

ℹ️ tests/loader/ isn't wired into CI

tests/loader/run-matrix.sh is the only thing in the repo that packs the tarballs and installs them into a clean directory, and tests/loader/README.md says plainly that install-from-tarball is what catches addon-resolution bugs — "one shipped that way in the first cut." But nothing under .github/workflows/ references tests/loader or run-matrix, so the first real staging of these packages happens during a release.

The compat-tier defect flagged inline is exactly the shape this matrix would catch, and it needs a Node in the 18.19–22.14 band to surface. Worth at least a Linux leg on PRs touching runtime/** or npm/loader*/**.

ℹ️ Nitpicks

  • scripts/build-loader-npm.mjs — the comment says the manifest cross-check catches drift "or vice versa", but RUNTIME_FILES.filter((f) => !manifest.files.includes(f)) only runs one direction. The reverse case (an entry dropped from RUNTIME_FILES but still listed in files[]) is the one that silently produces a broken tarball. A naive reverse check would false-positive on README.md/LICENSE, so correcting the comment is probably the cleaner fix.
  • site/content/docs/loader.mdx documents the .yaml/.toml/.json5/.jsonc/.txt imports and Worker, which is exactly the set site/content/docs/runtime/loaders.mdx fronts with <TypesSetup>. The loader page omits it, so a reader following it gets type errors on the imports it just showed them.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread runtime/loader-entry.mjs
Comment thread scripts/npm-first-publish-loader.sh Outdated
Comment thread runtime/loader-platform.cjs Outdated
Comment thread scripts/build-loader-npm.mjs Outdated
…, addon-path precedence, publish hardening

- The compat tier's loader worker imports its own transform-core instance, so
  the entry's CLOBBER_MAP.clear() never reached the map its resolveSpec reads:
  a user's real @js-temporal/polyfill silently bound undefined on 18.19-22.14
  (verified on 20.19). The clear now rides module.register's data payload into
  initialize(); the CLI registers with no data and keeps its clobbers.
- ensureAddonEnv no longer trusts an inherited __NUB_ADDON_PATH over its own
  platform-package resolution (version-skew hazard for a nested loader).
- npm-first-publish-loader.sh: npm >= 11.10 floor, and the trust step verifies
  via npm trust list instead of swallowing failures.
- release.yml: loader packages publish FIRST (before @nubjs/nub's irreversible
  publishes) and both publish jobs preflight that the nine packages exist.
- build-loader-npm.mjs: file: URL for the dynamic import (Windows).
- tests/loader: clobber.ts differential fixture (the defect above).

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ All four inline findings from the previous review are fixed; I verified each against the code rather than the commit message and retired the threads. Two diagnosability nitpicks inline, nothing blocking.

Reviewed changes

  • Re-derived the compat-tier clobber-map fix end to end: registerLoaderWorker (runtime/preload-common.cjs:370) forwards options untouched into module.register, both the ESM entry and the ERR_REQUIRE_ESM fallback send { standaloneLoader: true }, and the CLI's two call sites still pass nothing so their clobbers stay intact. The new clobber.ts fixture is non-vacuous — without the worker-side clear the compat tier prints undefined, not number.
  • Checked the initialize(data) ordering claim against Node's source rather than the comment it rests on. module.register() blocks the calling thread on Atomics.wait until the worker's Hooks#register has awaited initialize, so the clear lands before the first resolve. Confirmed at the v18.19.0 tag, where the 3-arg data form is also documented added: v18.19.0 — present at the support floor, not a later addition.
  • Traced the reworked ensureAddonEnv precedence across the realm matrix (fast tier, loader worker, user worker_threads, --require fallback, nested CLI in both directions). A worker's process.env is a copy taken at creation, and loader-addon-env.mjs is the first static import of loader-entry.mjs, so the var is always set before any worker exists — no race on either tier.
  • Walked the release reorder: preflight → loader platform packages → loader root → @nubjs/nub → types, mirrored in the canary job. The two copies are in sync, if ! npm view is correctly exempt from the runner's default bash -e so the loop accumulates all nine names instead of aborting on the first miss, and npm/nub carries no dependency on the loader family — so the new "loader publishes, @nubjs/nub fails" shape is a re-runnable partial publish rather than a version skew. Materially better than the previous ordering.
  • Confirmed the rename to @nubjs/loader is complete: no unscoped nubjs package reference survives anywhere in the tree, run-matrix.sh's tarball-name derivation still yields the right filename for a scoped name, and make version-check is green at 0.8.1 across all eleven manifests.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread runtime/loader-platform.cjs Outdated
Comment thread tests/loader/README.md
… the harness README

The last-resort branch that binds a parent process's __NUB_ADDON_PATH is the
one path that can pair version-skewed addon and JS, so it now says so on
stderr instead of succeeding silently.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

d1ab23f is the only commit since the last review, and it closes both nitpicks from it. I verified each against the code rather than the commit message, and retired both threads.

  • Warned on the inherited-addon fallbackruntime/loader-platform.cjs:92 now writes a stderr line naming the inherited path before returning true, so the one branch that produces a wrong-but-working addon no longer leaves zero trace.
  • Listed clobber.ts in the harness fixture tabletests/loader/README.md:21, the fixture whose purpose was least guessable from its filename.

Two things I checked rather than assumed about the new warning. It cannot fire spuriously: __NUB_ADDON_PATH is written in exactly one place in the tree (loader-platform.cjs:89) and the Nub CLI never sets it, so an inherited value always originates from another standalone-loader process — precisely the version-skew case the message names. And it cannot double-print on the compat tier: the loader worker imports preload-async-hooks.mjs directly and never reaches ensureAddonEnv, so the only repetition is one line per user worker thread that re-evaluates the entry, which is the same shape the total-failure branch below it already had.

Worth noting for future runs: run_one in tests/loader/run-matrix.sh captures 2>&1, so either warning firing during the matrix surfaces as a fixture mismatch rather than passing quietly.

Pullfrog  | View workflow run | Using Claude Opus𝕏

@colinhacks
colinhacks merged commit 0111d6e into main Sep 1, 2026
71 checks passed
@colinhacks
colinhacks deleted the loader branch September 1, 2026 00:04
@colinhacks

Copy link
Copy Markdown
Contributor Author

Shipped in v0.8.2: https://github.com/nubjs/nub/releases/tag/v0.8.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants