feat: publish browser build separately - #294
Draft
DecimalTurn wants to merge 2 commits into
Draft
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Browser publishing must remain retryable when the stable publish step has already completed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Separates the minified browser bundle from the standard npm package and publishes it under the browser dist-tag.
Changes:
- Adds separate main and browser builds.
- Stages and publishes a browser-only package.
- Adds browser smoke tests and updates documentation and dependencies.
File summaries
| File | Summary |
|---|---|
tsdown.config.ts |
Configures main and minified browser builds. |
scripts/prepare-browser-package.mjs |
Creates the browser package staging directory. |
README.md |
Updates the browser CDN example. |
pnpm-lock.yaml |
Removes the unused dependency. |
package.json |
Updates build scripts and package inclusion. |
CHANGELOG.md |
Documents the browser distribution. |
browser-tests/browser-build-smoke.spec.ts |
Adds browser bundle smoke tests. |
.github/workflows/publish.yml |
Publishes the browser package separately; moderate issue (3 votes): reruns may skip browser publishing after stable publication succeeds. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (3)
.github/workflows/publish.yml:59
- A failure in this new publish step also prevents the existing JSR release from running: the later
if: success() || steps.pnpm-publish.outcome == 'failure'is false when this step fails afterpnpm-publishsucceeded. That means a transient browser/npm error leaves npm's main package published but skips JSR; update the JSR condition or otherwise make this release failure path preserve the previously independent JSR publication.
run: npm publish ./dist/browser-package --tag browser --provenance --ignore-scripts
README.md:39
- The checked-in production demo still imports
https://unpkg.com/@decimalturn/toml-patch/dist/toml-patch.js, andscripts/build-demo.mjsdoes not emit the new@browserURL, so the CDN demo does not exercise the minified browser package introduced here. Please update the demo source/generator and regenerate the artifact so the CDN example consistently uses the browser dist-tag.
import * as TOML from 'https://unpkg.com/@decimalturn/toml-patch@browser/dist/browser/toml-patch.js';
package.json:99
- This allowlist is the core guarantee that the normal package omits
dist/browser, but the existing pack check only verifies that a tarball exists and the new tests inspect the build tree directly. Add a CI assertion over the packed file list thatdist/browser/toml-patch.jsis absent whiledist/toml-patch.jsremains present, otherwise this publish-safety behavior can regress unnoticed.
"dist/toml-patch.*"
- Files reviewed: 7/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+54
to
+55
| if: steps.pnpm-publish.outcome == 'success' | ||
| run: pnpm run prepare:browser-package |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary