Skip to content

fix(ci): green remaining node:typecheck/test reds (file-processing, media-processing, feature-flags sdk) - #94

Merged
acamarata merged 1 commit into
fix/nself-ci-node-typecheckfrom
fix/nself-ci-remaining-reds
Sep 12, 2026
Merged

acamarata merged 1 commit into
fix/nself-ci-node-typecheckfrom
fix/nself-ci-remaining-reds

Conversation

@acamarata

Copy link
Copy Markdown
Contributor

Summary

Stacks on top of #93 (targets its branch, not main) and fixes the three
node gate reds it explicitly left out of scope, plus one it exposed once
installs actually ran.

  • free/file-processing/ts and free/media-processing/ts: every
    "missing" module (commander, fastify, @types/node, @nself/plugin-utils)
    was already declared in package.json — never installed. Added the same
    pnpm install --frozen-lockfile --dir <pkg> step fix(ci): green node:typecheck for plugins-registry and feature-flags sdk #93 introduced, for
    both packages, plus for shared (their @nself/plugin-utils dependency
    is file:../../../shared; pnpm links it but never runs its build, so a
    pnpm --dir shared run build step was added ahead of the gate to
    produce the dist/ output the two plugins' types/imports resolve against).
  • Once file-processing could actually resolve @nself/plugin-utils, a
    real bug surfaced: src/server.ts imports createMetrics, which never
    existed in the shared package. Added a minimal, dependency-free
    Prometheus-text counter (shared/src/metrics.ts) matching the
    incrementRequest/incrementError/format surface already called.
  • free/feature-flags/sdk-ts: ts-jest was an installed devDependency
    with no jest.config at all, so jest ran with zero TS transform. Added
    jest.config.js (ts-jest preset + a moduleNameMapper stripping the
    NodeNext-style .js import extension back to .ts for resolution).
    That surfaced two more real gaps: the NodeNext/hybrid module kind needs
    isolatedModules: true for ts-jest's per-file transpile (ts-jest warned
    directly), and the test's global.fetch mock needed @types/node
    (never a declared dependency in that package).

No gate weakening anywhere: no skipped/disabled tests, no @ts-ignore/
as any, no excluded files, no ratchets raised.

Test plan

  • pnpm install --frozen-lockfile --dir <pkg> for shared,
    file-processing, media-processing, feature-flags/sdk-ts,
    .workers/plugins-registry — all succeed against the committed
    lockfiles
  • scripts/nself-ci.sh --check --no-gitleaks -v . reports PASS for
    every node:typecheck, node:test, and node:build entry across
    all 5 discovered workspace members — Overall: PASSED

…e jest transform for feature-flags sdk

nself-ci's node gate surfaced three more pre-existing reds beyond PR #93's
scope (same root cause class: nothing installs a nested package's own
dependencies before typecheck/test/build runs):

- free/file-processing/ts and free/media-processing/ts both had every
  declared dependency (commander, fastify, @types/node, @nself/plugin-utils,
  etc.) present in package.json but never installed. Added the same
  `pnpm install --frozen-lockfile --dir <pkg>` pattern PR #93 introduced.
  Their @nself/plugin-utils dependency is a file: path to shared/, whose
  dist/ output pnpm never builds on link — added a build step for shared
  ahead of the gate so the type/JS output exists.
- Installing file-processing surfaced a genuine bug once tsc could actually
  resolve @nself/plugin-utils: src/server.ts imports createMetrics, which
  was never implemented in the shared package. Added a minimal
  dependency-free Prometheus-text-format counter (shared/src/metrics.ts)
  matching the incrementRequest/incrementError/format surface the plugin
  already calls.
- free/feature-flags/sdk-ts had ts-jest installed but no jest config at
  all, so jest ran with zero TypeScript transform. Added jest.config.js
  (ts-jest preset + a moduleNameMapper for the NodeNext-style `.js` import
  extensions). That surfaced two more real gaps: the tsconfig's
  Node16/NodeNext module kind needs isolatedModules for ts-jest's per-file
  transpile, and the test file's `global.fetch` mock needed @types/node
  (never a declared dependency here).

scripts/nself-ci.sh --check --no-gitleaks -v . now reports PASS for every
node:typecheck, node:test, and node:build entry across all 5 discovered
workspace members.
@acamarata
acamarata merged commit 5f18297 into fix/nself-ci-node-typecheck Sep 12, 2026
5 checks passed
acamarata added a commit that referenced this pull request Sep 12, 2026
…sdk (#93)

* fix(ci): green the node:typecheck gate for plugins-registry and feature-flags sdk

nself-ci's new implicit-workspace-member discovery (gate_workspace.go, #91)
found and ran node:typecheck against .workers/plugins-registry and
free/feature-flags/sdk-ts for the first time, and both failed on main.

.workers/plugins-registry already declared @cloudflare/workers-types
correctly in package.json/tsconfig and had a valid committed lockfile — the
TS2688 only happened because nself-ci.yml never installed this directory's
dependencies before running its typecheck script, so tsc fell back to the
runner's global toolchain with no project node_modules at all. Same root
cause produced the sdk-ts TS2307 on 'react'. Added an install step for both
nested packages (this repo has no pnpm-workspace.yaml, so pnpm never
recurses into them on its own) and committed a lockfile for sdk-ts, which
had none.

Separately, sdk-ts's evaluate() typed its ctx parameter as the full
EvaluateRequest even though flag_key is always supplied by the explicit key
argument, never by ctx — evaluateFlag()/useFlag() correctly pass
Omit<EvaluateRequest, 'flag_key'>, which the wider parameter type rejected
(TS2345). Narrowed evaluate()'s ctx type to match what it actually needs,
and swapped the request-body spread order so an explicit key can never be
silently overridden by a flag_key on ctx.

free/file-processing/ts, free/media-processing/ts, and the sdk-ts jest
config remain red for unrelated pre-existing reasons and are out of scope
here.

* fix(ci): install+build deps for file-processing/media-processing, wire jest transform for feature-flags sdk (#94)

nself-ci's node gate surfaced three more pre-existing reds beyond PR #93's
scope (same root cause class: nothing installs a nested package's own
dependencies before typecheck/test/build runs):

- free/file-processing/ts and free/media-processing/ts both had every
  declared dependency (commander, fastify, @types/node, @nself/plugin-utils,
  etc.) present in package.json but never installed. Added the same
  `pnpm install --frozen-lockfile --dir <pkg>` pattern PR #93 introduced.
  Their @nself/plugin-utils dependency is a file: path to shared/, whose
  dist/ output pnpm never builds on link — added a build step for shared
  ahead of the gate so the type/JS output exists.
- Installing file-processing surfaced a genuine bug once tsc could actually
  resolve @nself/plugin-utils: src/server.ts imports createMetrics, which
  was never implemented in the shared package. Added a minimal
  dependency-free Prometheus-text-format counter (shared/src/metrics.ts)
  matching the incrementRequest/incrementError/format surface the plugin
  already calls.
- free/feature-flags/sdk-ts had ts-jest installed but no jest config at
  all, so jest ran with zero TypeScript transform. Added jest.config.js
  (ts-jest preset + a moduleNameMapper for the NodeNext-style `.js` import
  extensions). That surfaced two more real gaps: the tsconfig's
  Node16/NodeNext module kind needs isolatedModules for ts-jest's per-file
  transpile, and the test file's `global.fetch` mock needed @types/node
  (never a declared dependency here).

scripts/nself-ci.sh --check --no-gitleaks -v . now reports PASS for every
node:typecheck, node:test, and node:build entry across all 5 discovered
workspace members.

* fix(ci): build shared before installing its dependents

The nself-ci gate still failed with 'Cannot find module @nself/plugin-utils'
across every importing file in free/file-processing/ts and
free/media-processing/ts, even though shared was installed and built.

Cause: pnpm does not symlink a file: dependency back to the source tree. It
takes a real directory COPY into its store at install time. Building shared
afterwards populates shared/dist in the source tree but not in the copy the
dependents resolve against, and @nself/plugin-utils' main/types point at
./dist/*, so tsc finds no module.

Reordered so shared is built before file-processing and media-processing are
installed, which means their copies already contain dist/. Verified from a
clean state (dist and node_modules removed): both typechecks pass.

No gate was weakened and no dependency changed - only step ordering.
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.

1 participant