Skip to content

Add npm build test to CI and fix broken build#14

Draft
Copilot wants to merge 5 commits intomainfrom
copilot/test-npm-build-ci
Draft

Add npm build test to CI and fix broken build#14
Copilot wants to merge 5 commits intomainfrom
copilot/test-npm-build-ci

Conversation

Copy link
Contributor

Copilot AI commented Nov 29, 2025

The npm build (deno task build:dist) was only run during publish, meaning build failures wouldn't be caught until release time.

Changes

  • Added npm_build job to .github/workflows/test.yml with a Node.js smoke test to verify the artifact loads correctly
  • Aligned configuration with publish.yml (same Deno version, setup action, permissions)
  • Fixed pre-existing npm build failures:
    • Added a JSR resolver esbuild plugin to bundle @cross/runtime directly (instead of externalizing it, which would break npm consumers)
    • Created npm.ts as a Node.js-only entry point so only the Node.js shim is bundled (avoids including bun:test/Deno-specific static imports that would fail at load time in Node.js)
    • Removed CJS build (top-level await not supported in CommonJS format)
    • Removed dts-plugin (JSR type declarations unavailable during npm build)
    • Updated build/package.template.json to ESM-only exports referencing dist/npm.js
    • Excluded npm.ts from JSR publish
npm_build:
  runs-on: ubuntu-latest
  permissions:
    contents: read
  steps:
    - uses: actions/checkout@v4
    - name: Setup Deno
      uses: denoland/setup-deno@v1
      with:
        deno-version: v2.x
    - name: Build npm package
      run: deno task build:dist
    - name: Setup Node.js
      uses: actions/setup-node@v4
      with:
        node-version: "lts/*"
    - name: Smoke test Node artifact
      run: node -e "import('./dist/npm.js')"
Original prompt

Test npm build in ci, if it's not already done


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits November 29, 2025 01:05
Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Copilot AI changed the title [WIP] Add npm build test to CI pipeline Add npm build test to CI Nov 29, 2025
Copilot AI requested a review from Hexagon November 29, 2025 01:09
@Hexagon
Copy link
Member

Hexagon commented Nov 29, 2025

@copilot new build step actually failed, have a look

…build

Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 29, 2025

@copilot new build step actually failed, have a look

Fixed in e42a035. The npm build was failing due to two issues:

  1. @cross/runtime was not in esbuild's externals list
  2. CJS build was failing because esbuild doesn't support top-level await in CommonJS format

The fix removes the CJS build (ESM-only now) and adds @cross/runtime to externals. Also removed the dts-plugin since JSR type declarations aren't available during the npm build.

Copilot AI changed the title Add npm build test to CI Add npm build test to CI and fix broken build Nov 29, 2025
@Hexagon Hexagon requested a review from Copilot March 1, 2026 23:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds an npm build step to CI (so deno task build:dist failures are caught before release) and updates the build/package configuration to make the npm artifact ESM-only and avoid the previous build-time failures.

Changes:

  • Added an npm_build job to .github/workflows/test.yml to run deno task build:dist on PRs/pushes.
  • Updated build/build.ts to produce only an ESM Node bundle and to adjust esbuild externals.
  • Updated build/package.template.json to publish ESM-only exports for the generated npm package.json.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/test.yml Adds an npm build job to CI to run deno task build:dist.
build/build.ts Removes CJS + d.ts generation and switches to a single ESM build with updated externals.
build/package.template.json Updates npm package metadata/exports to be ESM-only.
Comments suppressed due to low confidence (1)

build/build.ts:70

  • The build command path no longer generates type declarations (the dts plugin/CJS build were removed), but the surrounding comment still says “Transpile and generate typings”. Updating this wording will help keep the build script’s guidance accurate.
  /* Handle argument `build`: Transpile and generate typings */
} else if (command === "build") {
  // Build the ESM JavaScript bundle

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

build/build.ts Outdated
Comment on lines +81 to +82
// @cross/runtime is a JSR dependency that must be externalized
external: ["bun:test", "node:test", "@cross/runtime"],
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

@cross/runtime is now marked as an esbuild external, which means the generated dist/mod.js will keep import "@cross/runtime" at runtime. The npm package template currently doesn’t declare any dependency/peerDependency on @cross/runtime, so consumers installing from npm will likely hit a Cannot find package '@cross/runtime' error unless they manually install it. Consider either bundling @cross/runtime (remove it from external) or adding it to the generated package.json (dependencies/peerDependencies) with a version aligned to deno.json.

Suggested change
// @cross/runtime is a JSR dependency that must be externalized
external: ["bun:test", "node:test", "@cross/runtime"],
external: ["bun:test", "node:test"],

Copilot uses AI. Check for mistakes.
with:
deno-version: v2.x
- name: Build npm package
run: deno task build:dist
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The new npm_build job only checks that deno task build:dist completes. It doesn’t validate that the produced Node artifact can actually be loaded (e.g., a simple node -e "import('./dist/mod.js')" or importing the generated package entry). Adding a minimal runtime smoke-check here would catch externals/dependency mismatches that still allow bundling to succeed.

Suggested change
run: deno task build:dist
run: deno task build:dist
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: 'lts/*'
- name: Smoke test Node artifact
run: node -e "import('./dist/mod.js')"

Copilot uses AI. Check for mistakes.
@Hexagon
Copy link
Member

Hexagon commented Mar 2, 2026

@copilot apply changes based on the comments in this thread

Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 2, 2026

@copilot apply changes based on the comments in this thread

Applied both review suggestions in 41563b6:

  1. @cross/runtime is now bundled (not external): Added a JSR resolver esbuild plugin that resolves @cross/runtime via Deno's module cache. Also created npm.ts as a Node.js-only entry point so only the Node.js shim is bundled (avoids including bun:test/Deno-specific imports that would fail at load time).

  2. Smoke test added to the CI job: node -e "import('./dist/npm.js')" to verify the artifact loads correctly in Node.js.

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