Skip to content

fix: resolve optionally declared upstream assets from the machine package root - #208

Merged
rponeawa merged 1 commit into
hypit-ai:mainfrom
daodaobing:fix/optional-dependencies
Sep 16, 2026
Merged

rponeawa merged 1 commit into
hypit-ai:mainfrom
daodaobing:fix/optional-dependencies

Conversation

@daodaobing

Copy link
Copy Markdown
Contributor

Closes #206

What this fixes

fonts-open declares all 109 bundled @fontsource* assets in optionalDependencies, because a Distribution ships them from the machine package root rather than through its own node_modules. declaredExternalPackageRoot read only dependencies, so the version lookup always returned undefined and the asset was reported as missing even right after a successful install:

$ hypit check <source>
x Source is invalid — @fontsource-variable/inter is needed by this authored font.
  Install it once with: hypit packages install @fontsource-variable/inter@5.3.0

$ hypit packages install @fontsource-variable/inter@5.3.0
$ hypit packages status  @fontsource-variable/inter@5.3.0     →  Ready true
$ hypit check <source>
x Source is invalid — @fontsource-variable/inter is needed by this authored font.
  Install it once with: hypit packages install @fontsource-variable/inter@5.3.0

The documented repair therefore never converged. This change falls back to optionalDependencies when the manifest does not list the name under dependencies.

Only an npm install exposes the bug. A contributor checkout links the fonts into packages/fonts-open/node_modules, and locateNodePackage returns from its nearby short circuit before reaching the external branch.

The change

-      const value = JSON.parse(readFileSync(manifest, "utf8")) as { dependencies?: Record<string, string> };
-      const version = value.dependencies?.[name];
+      const value = JSON.parse(readFileSync(manifest, "utf8")) as {
+        dependencies?: Record<string, string>;
+        optionalDependencies?: Record<string, string>;
+      };
+      const version = value.dependencies?.[name] ?? value.optionalDependencies?.[name];

Scope

  • location.ts L84-85 is the only place in the repository that reads dependencies to select an external package version (verified by grep), so this is one lookup, not a refactor.
  • dependencies still wins when a name appears in both, so no existing resolution order changes.

Test evidence

New regression case in packages/package-loader-node/test/location.test.ts:

$ node --import tsx --test packages/package-loader-node/test/location.test.ts
# tests 4 · pass 4 · fail 0

It fails on the unmodified source, which is what makes it a regression test:

$ git stash push -- packages/package-loader-node/src/location.ts
$ node --import tsx --test packages/package-loader-node/test/location.test.ts
not ok - ... declares an optionally installed upstream asset ...
$ git stash pop

End-to-end effect against a real npm Distribution Layout:

Call Before After
declaredExternalPackageRoot(<machine root>, <npm fonts-open/src/surface.ts>, "@fontsource-variable/inter") undefined <HYPIT_STATE_HOME>/packages/@fontsource-variable/inter/5.3.0
locateNodePackage(...) FAILED: cannot locate installed package LOCATED …/node_modules/@fontsource-variable/inter

Verification

  • pnpm check (repo-wide tsc --noEmit) — passes on this branch.
  • pnpm test — unchanged; the single failure is a pre-existing Windows EPERM in packages/compiler-node/test/compiler.test.ts when it creates a symlink escape sample. It reproduces on pristine main with this change stashed, and needs developer mode or an elevated shell.

This PR is independent of the Distribution child-process resolver fix (#207); the two touch disjoint files.

fonts-open declares 109 bundled @fontsource assets in optionalDependencies, but
declaredExternalPackageRoot read only dependencies, so every documented
'hypit packages install <name>@<version>' repair left the asset unreachable.

Read optionalDependencies as the fallback and cover it with a locator test.
@rponeawa

Copy link
Copy Markdown
Member

Thank you for the report and the fix.

Reproduced on the published 0.1.10 distribution (macOS, outside any checkout), following the CLI's own repair instruction:

hypit check repro/font.svml
  x @fontsource-variable/inter is needed by this authored font. Install it once with:
    hypit packages install @fontsource-variable/inter@5.3.0
hypit packages install @fontsource-variable/inter@5.3.0   -> Machine package is ready
hypit packages status  @fontsource-variable/inter@5.3.0   -> Ready true
hypit check repro/font.svml
  x same error

Applying this change to that same installed distribution and toggling it back and forth attributes the behavior to the one lookup:

distribution state hypit check
as published fails
with this change Source is valid
reverted fails
re-applied Source is valid

Your claim that location.ts holds the only external version lookup checks out, and dependencies keeping precedence means no existing resolution order moves. The new case fails with only src/location.ts reverted, so it holds the behavior rather than restating it.

Linux and Windows checks pass. Merging.

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.

Every bundled @fontsource* font is unreachable from the npm distribution: the machine-npm fallback reads only dependencies, not optionalDependencies

2 participants