Conversation
There was a problem hiding this comment.
Pull request overview
This pull request removes the "web" package from the esm.sh repository, which previously provided a no-build HTTP handler for serving web applications with HMR and on-the-fly transformation support. The PR refocuses the CLI tool exclusively on importmap management and modernizes the server codebase with Go 1.26 features.
Changes:
- Removed the entire
webpackage including handler, worker, and all template files - Removed CLI commands for
init,serve, anddevthat depended on the web package - Modernized server code to use Go 1.26 features (strings.CutPrefix, strings.SplitSeq, bytes.SplitSeq, range over integers, maps.Copy, fmt.Appendf)
Reviewed changes
Copilot reviewed 102 out of 148 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| web/* | Removed entire web package directory including handler, worker, utilities, and internal JavaScript files |
| cli/templates/* | Removed all project template files for various frameworks (React, Vue, Svelte, Preact, vanilla, with TailwindCSS/UnoCSS variants) |
| cli/command_*.go | Removed init, serve, and dev command implementations |
| cli/cli.go | Removed command registrations for init, serve, and dev |
| cli/utils.go | Removed embed import and walkEmbedFS utility function |
| cli/README.md, cli/npm/README.md, CHANGELOG-CLI.md | Updated documentation to reflect CLI scope reduction to importmap management only |
| cli/npm/package.json | Updated package description to reflect new scope |
| go.mod, go.sum | Removed gorilla/websocket dependency |
| server/*.go | Modernized code with Go 1.26 features: strings.CutPrefix, strings.SplitSeq, bytes.SplitSeq, range over integers, maps.Copy, fmt.Appendf |
| server/utils.go | Removed unused utility functions isAbsPathSpecifier and isLocalhost |
| Makefile | Removed make targets for CLI init, dev, and serve commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 96 out of 141 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
.github/workflows/release.yml:66
- The Windows build configuration has issues: 1) The matrix ext field is set to '' but should be '.exe' for Windows builds. 2) The build output path on line 56 is hardcoded to 'esm.sh' without using the extension variable, so Windows builds won't create 'esm.sh.exe' as expected. The build command should be
-o cli/npm/tmp/bin/esm.sh${{ matrix.ext }}and the upload path should becli/npm/tmp/bin/esm.sh${{ matrix.ext }}to properly handle Windows executables.
- os: windows
arch: arm64
ext: ''
- os: windows
arch: amd64
ext: ''
permissions:
contents: read
id-token: write
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Setup Go
uses: actions/setup-go@v5
with:
go-version: 1.26.x
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: 22
registry-url: 'https://registry.npmjs.org'
- name: Build CLI
run: go build -ldflags="-s -w" -o cli/npm/tmp/bin/esm.sh main.go
env:
GOOS: ${{ matrix.os }}
GOARCH: ${{ matrix.arch }}
CGO_ENABLED: "0"
- name: Upload Artifact
uses: actions/upload-artifact@v4
with:
name: cli-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }}
path: cli/npm/tmp/bin/esm.sh
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| if (!once.oxide) { | ||
| once.oxide = import("npm:@esm.sh/oxide-wasm@0.1.4").then(({ init, extract }) => init().then(() => ({ extract }))); | ||
| once.oxide = import("npm:oxide-wasm@0.1.1").then(({ init, extract }) => init().then(() => ({ extract }))); |
There was a problem hiding this comment.
The package name has been changed from npm:@esm.sh/oxide-wasm@0.1.4 to npm:oxide-wasm@0.1.1. This appears to be a downgrade and a package name change. Please verify this is intentional and that oxide-wasm@0.1.1 provides the same API as @esm.sh/oxide-wasm@0.1.4, particularly the init and extract functions.
web/README.md
Outdated
| <body class="flex justify-center items-center h-screen"> | ||
| <div id="app"></div> | ||
| <script type="module" src="/app.tsx"></script> | ||
| <script type="module" src="./app.tsx"></script> |
There was a problem hiding this comment.
The script src was changed from "/app.tsx" to "./app.tsx" (absolute to relative). While this change is functionally equivalent in most cases, it represents an inconsistency with the actual example that follows, which should also be updated to use the relative path if this change is intentional. However, this appears to be an unintended change that's not related to the PR's purpose of removing CLI commands and UnoCSS support.
Uh oh!
There was an error while loading. Please reload this page.