Skip to content

feat: ✨ add zsh dialect support (rebuild wasm against mvdan/sh v3.13.1) - #136

Merged
JounQin merged 9 commits into
un-ts:mainfrom
jjloneman:feat/gh-135-add-zsh-support
Jul 6, 2026
Merged

JounQin merged 9 commits into
un-ts:mainfrom
jjloneman:feat/gh-135-add-zsh-support

Conversation

@jjloneman

@jjloneman jjloneman commented Jul 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #135

🤖 Disclaimer: I had Claude help me create this PR and generate the PR description.

🐛 What this fixes

🧪 Live demo (before / after)

Both run npm start in an isolated container (each build registers its own globalThis.Go, so they're kept in separate sandboxes):

  • 🔴 Before — [email protected], errors on valid zsh:
    Edit sh-syntax-zsh-before
  • 🟢 After — this PR's build, zsh parses & prints (${0:A:h:h}):
    Edit sh-syntax-zsh-after

🛠️ Changes

  • go.mod — bump mvdan/sh v3.11.0 → v3.13.1 (requires Go 1.25).
  • src/types.ts — LangVariant became a bit-flag enum in mvdan/sh v3.13; update all values and add LangZsh.
  • main.go + processor/structs_easyjson.go — marshal via jwriter directly to drop the net/http pull-in that TinyGo can't compile for js/wasm.
  • vendors/wasm_exec.cjs — regenerate to match the TinyGo used for the rebuild (a stale runtime silently corrupts wasm memory).
  • main.wasm — rebuilt with TinyGo 0.41.1 + Go 1.25.
  • tests — add test/zsh.spec.ts; update snapshots for mvdan/sh v3.13's backtick-wrapped error tokens.

⚠️ Breaking change

  • LangVariant numeric values changed from a sequential enum to bit flags (1 << iota) to match mvdan/sh v3.13:
    • LangBash: 0 → 1
    • LangPOSIX: 1 → 2
    • LangMirBSDKorn: 2 → 4
    • LangBats: 3 → 8
    • LangZsh: (new) → 16
    • LangAuto: 4 → 32
  • Code using the exported LangVariant constants is unaffected; only code passing raw variant numbers needs updating.

📦 Versioning

  • Included a minor changeset (→ 0.6.0), since the breaking LangVariant renumber lands in the minor slot for a pre-1.0 package.

✅ Testing

  • yarn vitest run → 4 files / 7 tests passing, including the 3 new zsh tests, zero memory-corruption errors.
  • Verified both in a clean Linux container and with the container-built main.wasm running under Node on macOS.

📝 Notes for reviewers

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Added Zsh dialect support, including successful parsing and accurate printing of Zsh-specific constructs.
  • Bug Fixes

    • Improved invalid-parse diagnostics by rendering the encountered token using backticks.
  • Breaking Change

    • Updated LangVariant to bit-flag values, added LangZsh, and changed the numeric values for existing variants—integrations using raw numeric values must be updated.
  • Chores

    • Refreshed the JS/WASM runtime shim for better Node compatibility and updated WASM build/tooling dependencies.

@changeset-bot

changeset-bot Bot commented Jul 5, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 7cda7c5

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

This PR includes changesets to release 1 package
Name Type
sh-syntax Minor

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

@codesandbox-ci

codesandbox-ci Bot commented Jul 5, 2026 •

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Upgrades mvdan.cc/sh/v3 to v3.13.1, updates LangVariant to bit flags with LangZsh, adds zsh regression coverage, switches wasm JSON marshalling to jwriter, and refreshes vendors/wasm_exec.cjs for updated Node, WASI, and runtime behavior.

Changes

Zsh dialect support and wasm rebuild

Layer / File(s) Summary
Dependency upgrade and changelog
go.mod, .changeset/gh-135-add-zsh-support.md
Bumps the Go toolchain to 1.25.0 and mvdan.cc/sh/v3 to v3.13.1; the changeset records zsh support and the breaking LangVariant value change.
LangVariant bit-flag conversion and zsh tests
src/types.ts, test/zsh.spec.ts, test/basic.spec.ts
Converts LangVariant constants to bit flags, adds LangZsh, adds zsh parsing/printing regression tests, and updates an error-message snapshot.
easyjson marshalling switched to jwriter
main.go, processor/structs_easyjson.go
Replaces easyjson.Marshal with jwriter.Writer/MarshalEasyJSON/BuildBytes and removes the unused easyjson import and suppression variable.
wasm_exec.cjs regeneration for updated Go/TinyGo runtime
vendors/wasm_exec.cjs
Updates Node polyfill imports to node:-prefixed modules, standardizes WASI syscall stub errno returns, and changes runtime.ticks/runtime.sleepTicks timing behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test as test/zsh.spec.ts
  participant Parser as parse()
  participant Printer as print()
  Test->>Parser: parse(ZSH_SNIPPET, LangZsh)
  Parser-->>Test: parsed AST
  Test->>Printer: print(AST, LangZsh)
  Printer-->>Test: reproduced snippet
  Test->>Parser: parse(ZSH_SNIPPET, LangBash)
  Parser-->>Test: error /ternary operator missing/
Loading

Possibly related PRs

  • un-ts/sh-syntax#107: Both PRs modify src/types.ts’s LangVariant; this PR redefines the enum as bit flags, updates LangAuto, and adds LangZsh.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: adding zsh dialect support and rebuilding against mvdan/sh v3.13.1.
Linked Issues check ✅ Passed The changes cover the parser upgrade, LangVariant updates, wasm/easyjson fixes, regenerated runtime glue, and zsh regression tests.
Out of Scope Changes check ✅ Passed The diff stays focused on zsh support and the wasm/toolchain updates needed to enable it.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size
lib/browser.js 99 B (0%)
lib/index.js 334 B (0%)

@socket-security

socket-security Bot commented Jul 5, 2026 •

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedgolang/​mvdan.cc/​sh/​v3@​v3.11.0 ⏵ v3.13.193100100100100

View full report

@codacy-production

codacy-production Bot commented Jul 5, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Fixes valid zsh scripts erroring with "ternary operator missing ? before :".
See un-ts#135.

- go.mod: bump mvdan/sh v3.11.0 -> v3.13.1 (requires Go 1.25)
- src/types.ts: LangVariant became a bit-flag enum in mvdan/sh v3.13;
  update all values and add LangZsh
- main.go / processor/structs_easyjson.go: marshal via jwriter to drop the
  net/http pull-in that TinyGo cannot compile for the js/wasm target
- vendors/wasm_exec.cjs: regenerate to match TinyGo 0.41.1; a stale runtime
  silently corrupts wasm linear memory
- rebuild main.wasm (TinyGo 0.41.1 + Go 1.25)
- add zsh regression test; update snapshots for backtick-wrapped error tokens

BREAKING CHANGE: LangVariant numeric values changed to bit flags (1 << iota);
code passing raw variant numbers (rather than the LangVariant constants) must update.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@jjloneman
jjloneman force-pushed the feat/gh-135-add-zsh-support branch from 7311757 to 97bc6f3 Compare July 5, 2026 05:13
@jjloneman

Copy link
Copy Markdown
Contributor Author

🤖 Disclaimer: I had Claude help me write this comment.

🔴 Why SonarCloud & Codacy are red

  • Both quality gates fail almost entirely on the vendored vendors/wasm_exec.cjs — the TinyGo js/wasm runtime glue, not hand-written code.
  • SonarCloud: 43 findings → 42 in vendors/wasm_exec.cjs, dropping new-code maintainability to B (gate requires A).
  • Codacy: "37 new issues (0 allowed)" — same vendored file.
  • The file has always had these smells (generated code: reassigned params, bitwise ops, ==, etc.); Sonar only grades new code, and since this PR regenerates the whole file (TinyGo 0.37 → 0.41.1) every line now counts as new — so its pre-existing smells trip the gate. Classic false positive for regenerated/vendored runtimes.

✅ The one hand-written finding is already fixed

  • The only non-vendored finding was src/types.ts:16 (Sonar S7767: Use Math.trunc instead of << 0) on LangBash: 1 << 0.
  • Fixed by writing LangBash: 1 (the << 0 was a literal no-op). Hand-written code now has zero findings.

🛠️ Suggested resolution (maintainer side)

  • These gates can't be turned green from the PR — the exclusions live in the un-ts SonarCloud/Codacy dashboards.
  • Options:
    • Exclude vendors/** from analysis (SonarCloud sonar.exclusions / Codacy ignore) — appropriate for vendored/generated runtime files.
    • Or waive/override the gate for this PR.
  • Happy to add a sonar-project.properties with sonar.exclusions=vendors/** if you'd prefer the exclusion committed to the repo — just let me know.

🟢 For reference — the checks that matter all pass

  • CI build + test, size-limit, CodeSandbox CI, and both Socket Security checks are green.

@jjloneman
jjloneman force-pushed the feat/gh-135-add-zsh-support branch from 97bc6f3 to 14ab712 Compare July 5, 2026 05:58

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
vendors/wasm_exec.cjs (1)

32-32: 📐 Maintainability & Code Quality | 🔵 Trivial

Static-analysis findings here are on vendored generated code — exclude vendors/** rather than hand-edit.

SonarCloud's global→globalThis and optional-chain suggestions target the regenerated TinyGo wasm_exec glue. Editing them would diverge from the upstream generator output and violate the "use the same wasm_exec as the TinyGo build" requirement, risking runtime incompatibility. Prefer excluding vendors/** from SonarCloud/Codacy (e.g. a sonar-project.properties with sonar.exclusions=vendors/**) so the gate reflects hand-written code only.

Also applies to: 538-544

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vendors/wasm_exec.cjs` at line 32, The reported `global`/optional-chain
findings are in vendored generated TinyGo glue (`wasm_exec`), so do not modify
`vendors/wasm_exec.cjs` or related generated symbols like the `global.fs` setup.
Instead, update the SonarCloud/Codacy configuration to exclude `vendors/**` so
only hand-written code is analyzed, keeping the upstream `wasm_exec` output
unchanged and avoiding divergence from the TinyGo-generated runtime.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@vendors/wasm_exec.cjs`:
- Line 32: The reported `global`/optional-chain findings are in vendored
generated TinyGo glue (`wasm_exec`), so do not modify `vendors/wasm_exec.cjs` or
related generated symbols like the `global.fs` setup. Instead, update the
SonarCloud/Codacy configuration to exclude `vendors/**` so only hand-written
code is analyzed, keeping the upstream `wasm_exec` output unchanged and avoiding
divergence from the TinyGo-generated runtime.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bc584878-d40c-4671-b938-2b0411cee3fc

📥 Commits

Reviewing files that changed from the base of the PR and between 7311757 and 14ab712.

⛔ Files ignored due to path filters (4)
  • go.sum is excluded by !**/*.sum
  • main.wasm is excluded by !**/*.wasm
  • test/__snapshots__/fixtures.spec.ts.snap is excluded by !**/*.snap
  • test/__snapshots__/processor.spec.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (8)
  • .changeset/gh-135-add-zsh-support.md
  • go.mod
  • main.go
  • processor/structs_easyjson.go
  • src/types.ts
  • test/basic.spec.ts
  • test/zsh.spec.ts
  • vendors/wasm_exec.cjs
💤 Files with no reviewable changes (1)
  • processor/structs_easyjson.go
✅ Files skipped from review due to trivial changes (2)
  • test/basic.spec.ts
  • .changeset/gh-135-add-zsh-support.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • test/zsh.spec.ts
  • main.go
  • go.mod
  • src/types.ts

Comment thread vendors/wasm_exec.cjs Resolved
Comment thread vendors/wasm_exec.cjs Resolved
Address review on un-ts#136: re-apply the local modifications that the raw TinyGo
output dropped, keeping the file matching the project's convention.

- restore the `// modified based on <tinygo>/targets/wasm_exec.js` header, with
  the link updated to the TinyGo v0.41.1 source used for the rebuild
- drop the Node CLI-runner block at the end (a local modification), so the file
  ends at `})();` as before

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@pkg-pr-new

pkg-pr-new Bot commented Jul 6, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/sh-syntax@136

commit: 7cda7c5

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the project’s embedded mvdan/sh parser and WASM runtime to add Zsh dialect support (fixing valid zsh like ${0:A:h:h} failing to parse) and aligns TypeScript/public APIs and tests with the newer parser behavior.

Changes:

  • Bump mvdan/sh to v3.13.1 (Go 1.25) and update exported LangVariant constants to the new bit-flag values, adding LangZsh.
  • Adjust WASM/Go JSON marshaling to avoid net/http (TinyGo js/wasm compatibility) and regenerate/update the WASM runtime shim.
  • Add Zsh regression tests and update snapshots for the new error-token formatting.

Reviewed changes

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

Show a summary per file
File Description
vendors/wasm_exec.cjs Regenerated TinyGo WASM runtime shim; updates Node built-in imports and WASI/timing glue.
main.go Switch JSON marshaling to jwriter to avoid TinyGo-incompatible net/http dependency path.
processor/structs_easyjson.go Removes unused easyjson package reference from generated file to prevent TinyGo pulls.
src/types.ts Updates LangVariant to bit flags and adds LangZsh.
test/zsh.spec.ts Adds regression coverage for parsing/printing Zsh parameter-expansion modifiers.
test/basic.spec.ts Updates inline snapshot to match new backtick-wrapped error token formatting.
test/__snapshots__/processor.spec.ts.snap Snapshot updates for new error token formatting.
test/__snapshots__/fixtures.spec.ts.snap Snapshot updates for new error token formatting.
go.mod Sets Go version to 1.25.0 and bumps mvdan.cc/sh/v3 to v3.13.1.
go.sum Updates dependency checksums for mvdan.cc/sh/v3 v3.13.1.
.changeset/gh-135-add-zsh-support.md Records the feature + breaking LangVariant renumbering for release automation.
Files not reviewed (1)
  • processor/structs_easyjson.go: Generated file

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

Comment thread vendors/wasm_exec.cjs Resolved
Comment thread src/types.ts Resolved
Comment thread test/zsh.spec.ts Resolved
jjloneman and others added 2 commits July 5, 2026 22:40
Address Copilot review on un-ts#136: the `LangVariant` type only exposes the named
bit-flag constants, so remove the doc claims that implied the legacy `0` value
is usable, and reword the `variant?` default note accordingly.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Address Copilot review on un-ts#136: point the reference at the specific issue
instead of the generic issues list.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@jjloneman

Copy link
Copy Markdown
Contributor Author

@JounQin, copilot made one erroneous comment which I replied to, but made two valid comments regarding minor jsdoc inconsistencies which I fixed. Do the changes look good to merge?

The quality gate is failing due to the generated vendor file, but we can't control those specific issues (they could be ignored with sonar.exclusions=vendors/** - see my other comment #136 (comment)).

@JounQin

JounQin commented Jul 6, 2026

Copy link
Copy Markdown
Member

@jjloneman lint still doesn't pass on CI.

jjloneman and others added 2 commits July 5, 2026 23:58
The `LangVariant` `1 << iota` values are intentional bit flags, so scope off
`@typescript-eslint/no-magic-numbers` for src/types.ts instead of an inline
disable on the `LangZsh` line.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
- switch the zsh parse assertion from the generic `.resolves.toBeTruthy()` to
  the more semantic `.resolves.toBeDefined()`
- convert the leading comment to a starred block to satisfy
  @stylistic/js/multiline-comment-style

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@jjloneman

Copy link
Copy Markdown
Contributor Author

@jjloneman lint still doesn't pass on CI.

@JounQin, ah you're right. It was complaining about magic numbers for the iota sequence of the LangVariant flags which I added an ignore config for (I hope that's alright - if you'd rather have me add eslint disable block comments within the file itself for that specific const, I can do that instead of the ignore config).

I also fixed the multiline -> block comment issue and ran yarn lint and yarn test locally which passed.

Would you mind running the CI workflows again? Thanks!

@codecov

codecov Bot commented Jul 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (d90f699) to head (7cda7c5).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #136   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines           77        77           
  Branches        29        29           
=========================================
  Hits            77        77           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread eslint.config.js Resolved
Per review on un-ts#136, prefer an in-file disable over editing the shared eslint config:

- add a scoped `@typescript-eslint/no-magic-numbers` disable block (with a `--` reason) around `LangVariant`
- revert the `src/types.ts` override in eslint.config.js

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

Comment thread src/types.ts Resolved
@JounQin
JounQin merged commit ecd5eb7 into un-ts:main Jul 6, 2026
25 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 6, 2026
@jjloneman
jjloneman deleted the feat/gh-135-add-zsh-support branch July 6, 2026 05:13
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.

[bug] 🐛 valid zsh errors with ternary operator missing ? before : — vendored mvdan/sh version has no zsh dialect

3 participants