Skip to content

Added knip to the test pipeline - #1764

Open
nico-martin wants to merge 2 commits into
mainfrom
nico/knip-pipeline
Open

nico-martin wants to merge 2 commits into
mainfrom
nico/knip-pipeline

Conversation

@nico-martin

@nico-martin nico-martin commented Sep 3, 2026 •

Copy link
Copy Markdown
Collaborator

second part of #1701 (1st Part is #1763 )

@nico-martin nico-martin changed the title Nico/knip pipeline Added knip to the test pipeline Sep 3, 2026
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@nico-martin nico-martin mentioned this pull request Sep 3, 2026
@Xxx91n

Xxx91n commented Sep 27, 2026

Copy link
Copy Markdown

This PR's packages/transformers/package.json hunk ("onnxruntime-common": "1.24.3") is the actual fix for the ghost dependency reported in #1087 — and it's still needed. Confirming with a runtime repro that the published 4.3.0 tarball still carries a top-level bare require("onnxruntime-common") with no manifest declaration.

Reproduction (published 4.3.0, pnpm isolated scopes)

Fixture: a consumer package whose only dependency is @huggingface/transformers@4.3.0, installed with pnpm's isolated layout (hoist: false in pnpm-workspace.yaml — node_modules/.pnpm/node_modules stays empty). Then:

$ node -e "require('@huggingface/transformers')"

Error: Cannot find module 'onnxruntime-common'
Require stack:
- …/node_modules/.pnpm/@huggingface+transformers@4.3.0_@types+node@26.6.2/node_modules/@huggingface/transformers/dist/transformers.node.cjs
    at Module._resolveFilename (node:internal/modules/cjs/loader:1420:15)
    …
    at Object.<anonymous> (…/transformers.node.cjs:13520:33)
  code: 'MODULE_NOT_FOUND'

Same result on 3.8.1 — the eager require predates the 4.x line. Node v24.11.0, pnpm 11.24.0, Windows 11.

One nuance worth stating precisely: pnpm's default config masks this — the virtual-store hoist (hoistPattern: * → .pnpm/node_modules) makes onnxruntime-common reachable anyway. The break surfaces wherever no shared hoist dir is reachable: isolated/strict scopes (hoist: false), pnpm dlx contexts (where packageExtensions can't apply either), and global-style installs. npm's flat hoisting likewise only masks it — and stops masking as soon as dependency resolution lands a conflicting copy.

Why downstream can't absorb this

Every consumer-side fix fails to travel with a published package:

So each downstream project has independently reinvented the same workaround:

That's at least four independent implementations of the same one-line manifest declaration — plus whatever private forks exist that never showed up in search.

Ask

Could this PR (or just the package.json declaration hunk) get merge priority? The declaration matches what src/backends/onnx.js actually imports, it's what onnxruntime-node/onnxruntime-web already do for their own dependency, and it would let every downstream workaround retire. Happy to re-verify against the merge result and report back on #1087.

@Xxx91n Xxx91n mentioned this pull request Sep 27, 2026
5 tasks

This branch has not been deployed

No deployments
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.

3 participants