Skip to content

Conversation

@threepointone
Copy link
Collaborator

We inline types from @repo/shared so that it includes the types we reexport. Fixes #165

Updated tsup to tsdown, and normalized some deps. Added typecheck steps to example repos (except basic, so that CI passes for this one). I'll do more monorepo maintenance after this change lands.

We inline types from `@repo/shared` so that it includes the types we reexport. Fixes #165

Updated tsup to tsdown, and normalized some deps. Added typecheck steps to example repos (except basic, so that CI passes for this one). I'll do more monorepo maintenance after this change lands.
@changeset-bot
Copy link

changeset-bot bot commented Oct 29, 2025

🦋 Changeset detected

Latest commit: 6132685

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cloudflare/sandbox Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@claude
Copy link
Contributor

claude bot commented Oct 29, 2025

Claude Code Review

Summary: This PR effectively fixes issue #165 by switching from tsup to tsdown and inlining types from @repo/shared. The approach is sound and aligns with the project's architecture.

Key Changes:

  • Migrated build tooling from tsup to tsdown
  • Added tsdown.config.ts with resolve: ['@repo/shared'] to inline shared types into the published package
  • Normalized dependencies across examples (using * for workspace packages, updated @types/node and wrangler versions)
  • Added typecheck scripts to example projects for better CI coverage
  • Updated changeset config to ignore @repo/* packages

Architecture Alignment: ✅
The solution correctly addresses the root cause: @repo/shared types were exported but not available to consumers since it's an internal monorepo package. Inlining these types ensures consumers get full type definitions without needing unpublished dependencies.

Issues:

  1. Missing tsdown dependency in @cloudflare/sandbox: The packages/sandbox/package.json uses tsdown in its build script but only has it listed as a root-level devDependency. This could cause issues if someone builds the package in isolation. Consider adding "tsdown": "^0.15.11" to packages/sandbox/devDependencies.

  2. Example basic excluded from typecheck: The PR description mentions "Added typecheck steps to example repos (except basic, so that CI passes for this one)". This suggests examples/basic has type errors that need fixing rather than being excluded. Technical debt should be tracked.

Minor Observations:

  • The changeset message could be slightly more descriptive about the tsup→tsdown migration for future reference
  • Dependency version updates (wrangler 4.42.2→4.44.0) bundled in this PR are reasonable but technically unrelated to the fix

Recommendation: Approve after addressing issue #1. Issue #2 is non-blocking but should be tracked separately.

Copy link
Member

@ghostwriternr ghostwriternr left a comment

Choose a reason for hiding this comment

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

lgtm, needs a minor change so we don't break c3 + tutorials

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 29, 2025

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/sandbox-sdk/@cloudflare/sandbox@168

commit: 6132685

@github-actions
Copy link
Contributor

🐳 Docker Image Published

FROM cloudflare/sandbox:0.0.0-pr-168-6b72c1a

Version: 0.0.0-pr-168-6b72c1a

You can use this Docker image with the preview package from this PR.

@threepointone threepointone merged commit 6b08f02 into main Oct 29, 2025
14 checks passed
@threepointone threepointone mentioned this pull request Oct 29, 2025
@threepointone threepointone deleted the fix-types branch November 16, 2025 17:33
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.

Type definitions from @repo/shared are not accessible to consumers

2 participants