Skip to content

fix: enforce direct runtime dependency declarations - #778

Merged
kang-heewon merged 1 commit into
trunkfrom
fix/562-dataloader-otel-dep
Jun 15, 2026
Merged

fix: enforce direct runtime dependency declarations#778
kang-heewon merged 1 commit into
trunkfrom
fix/562-dataloader-otel-dep

Conversation

@kang-heewon

Copy link
Copy Markdown
Member

Fixes #562.

Summary

The package manifest checker now audits non-test package source files for runtime external imports and requires each imported package to be declared directly in dependencies, peerDependencies, or optionalDependencies. @croco/dataloader-core already declares @opentelemetry/api; the new manifest gate prevents that packaging contract from regressing when a runtime import is only available through a transitive dependency.

변경 사항

  • Add a TypeScript AST-based source runtime import audit to scripts/normalize-packages.mjs.
  • Ignore type-only imports, tests, relative imports, package self-imports, Node builtins, and the existing specialized reflect-metadata policy.
  • Add a regression fixture that mirrors @croco/dataloader-core importing @opentelemetry/api while only declaring @croco/telemetry-api.

Verification

  • pnpm exec vitest run scripts/tests/normalize-packages.spec.ts --reporter verbose - passed, 10 tests.
  • pnpm package-manifests:check - passed, 97 checked / 2 private skipped.
  • pnpm changeset-required:check -- --base origin/trunk --head HEAD - passed, no publishable package behavior changes detected.
  • git diff --check - passed.
  • pnpm check - passed.
  • pnpm build - passed, 99 tasks.
  • pnpm package-entrypoints:smoke - passed, 96 public packages; @croco/dataloader-core resolved through CJS, ESM, and TypeScript consumers.
  • pnpm test - passed, 197 tasks.
  • pnpm typecheck - passed, 196 tasks.
  • Pre-push hook passed: auto-changeset skipped, full cached test 197/197 tasks, and full cached typecheck 196/196 tasks.

Self-review

  • Correctness/regression: PASS. @croco/dataloader-core declares @opentelemetry/api, the new manifest regression fails when a dataloader-like source value import is not declared directly, repo-wide manifest validation passes, and built package-entrypoint smoke imports @croco/dataloader-core through CJS, ESM, and TypeScript consumers.
  • API/security/compatibility/release: PASS. No public runtime API, package exports, package manifests, lockfile entries, secrets, or environment assumptions changed; the changeset gate confirms no publishable package behavior change.
  • Maintainability/minimality: PASS. The change stays inside the existing package manifest validator and regression suite, uses the TypeScript AST to avoid type-only false positives, and adds no dependency or unrelated cleanup.

Risk

Low. This only tightens repository manifest validation; packages that add new runtime external imports now need to declare them directly before pnpm package-manifests:check can pass.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@kang-heewon, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 55 minutes and 39 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6645dc02-b541-4d56-a875-d675294f74db

📥 Commits

Reviewing files that changed from the base of the PR and between 51b0f14 and 2134359.

📒 Files selected for processing (2)
  • scripts/normalize-packages.mjs
  • scripts/tests/normalize-packages.spec.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/562-dataloader-otel-dep

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

📊 Benchmark Results

❌ Some benchmarks failed

Gate failures

  • CrocoApp constructor: p75 3.2μs exceeds baseline 2.5μs by more than 20%
  • Lambda cold-start simulation: p75 177.2μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start with headers: p75 92.2μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start with binary body: p75 87.9μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start with query params: p75 79.6μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start with authorizer context: p75 81.8μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start realistic scenario: p75 83.5μs exceeds baseline 60.0μs by more than 20%
  • Container.get singleton (cold): p75 1.0μs exceeds baseline 0.7μs by more than 20%
  • Container.get singleton (warm): p75 0.4μs exceeds baseline 0.3μs by more than 20%
Benchmark p75 Threshold Baseline vs Baseline Status Notes
CrocoApp constructor 3.2μs 30.0ms 2.5μs +28.2% -
CrocoApp lambdaHandler (10 controllers) 32.9μs 50.0ms 30.0μs +9.8% -
Lambda cold-start simulation 177.2μs 80.0ms 60.0μs +195.4% -
Lambda cold-start with headers 92.2μs 80.0ms 60.0μs +53.6% -
Lambda cold-start with binary body 87.9μs 80.0ms 60.0μs +46.6% -
Lambda cold-start with query params 79.6μs 80.0ms 60.0μs +32.6% -
Lambda cold-start with authorizer context 81.8μs 80.0ms 60.0μs +36.4% -
Lambda cold-start realistic scenario 83.5μs 80.0ms 60.0μs +39.2% -
EventBusConfig.start (10 handlers) 1.5μs 10.0ms 9.0μs -83.9% -
EventPublisher.publishNow single event 1.7μs 2.0ms 1.5μs +12.8% -
DefaultHandlerResolver.resolve × 10 0.1μs 5.0ms 0.2μs -60.0% -
Container.get singleton (cold) 1.0μs 5.0ms 0.7μs +35.9% -
Container.register × 50 components 10.5μs 10.0ms 10.0μs +5.4% -
Container.validate (50 components) 27.7μs 20.0ms 25.0μs +10.8% -
Container.get singleton (warm) 0.4μs 500.0μs 0.3μs +23.7% -
TelemetryRuntime.init (lambda preset) 1.0ms 200.0ms 60.0ms -98.3% -
lambdaPreset config creation 1.4μs 2.0ms 2.0μs -31.3% -

Updated: 2026-06-15T05:45:28.347Z · Commit: a2a7850

@kang-heewon
kang-heewon merged commit 1dfea79 into trunk Jun 15, 2026
7 of 8 checks passed
@kang-heewon
kang-heewon deleted the fix/562-dataloader-otel-dep branch June 15, 2026 05:54
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.

[dataloader-core] Runtime OpenTelemetry API import is undeclared

1 participant