Skip to content

chore: sync tooling and linting with marko-js/prettier#30

Open
DylanPiercey wants to merge 1 commit into
mainfrom
claude/marko-testing-library-sync-pwdq86
Open

chore: sync tooling and linting with marko-js/prettier#30
DylanPiercey wants to merge 1 commit into
mainfrom
claude/marko-testing-library-sync-pwdq86

Conversation

@DylanPiercey

@DylanPiercey DylanPiercey commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Description

Modernizes @marko/testing-library's tooling to match marko-js/prettier:

  • ESLint flat config (eslint.config.mjs) with typescript-eslint + simple-import-sort
  • Prettier 3 + prettier-plugin-packagejson
  • Husky 9 and lint-staged (now covering .mjs)
  • Changesets for versioning/release (replaces standard-version, commitlint, fixpack)
  • rolldown for bundling (replaces esbuild)
  • Vitest for testing (replaces jest): server (node) and browser (jsdom) projects, with .marko compiled per-target via @marko/vite
  • Build/test/release CI workflow + dependabot, modern tsconfig
  • All dependencies upgraded to latest (Marko 5, jsdom 29, @testing-library/jest-dom 6, TypeScript 6, …); npm audit clean

Motivation and Context

Keeps tooling and linting consistent with the other marko-js repos and fully up to date.

Checklist:

  • I have updated/added documentation affected by my changes.
  • I have added tests to cover my changes.

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 15.78947% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.42%. Comparing base (7fdb637) to head (1592776).

Files with missing lines Patch % Lines
src/shared.ts 7.14% 13 Missing ⚠️
src/index-browser.ts 50.00% 0 Missing and 2 partials ⚠️
src/index.ts 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #30       +/-   ##
===========================================
- Coverage   82.00%   65.42%   -16.59%     
===========================================
  Files           3        3               
  Lines         239      188       -51     
  Branches       69       50       -19     
===========================================
- Hits          196      123       -73     
- Misses         42       44        +2     
- Partials        1       21       +20     

☔ 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.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 38 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2fd8c5b4-b4d1-4d53-9b5d-02f705f57286

📥 Commits

Reviewing files that changed from the base of the PR and between 9577926 and 1592776.

⛔ Files ignored due to path filters (4)
  • package-lock.json is excluded by !**/package-lock.json and included by **
  • src/__tests__/__snapshots__/render.browser.test.ts.snap is excluded by !**/*.snap, !**/__snapshots__/** and included by **
  • src/__tests__/__snapshots__/render.server.test.ts.snap is excluded by !**/*.snap, !**/__snapshots__/** and included by **
  • src/__tests__/__snapshots__/render.server.ts.snap is excluded by !**/*.snap, !**/__snapshots__/** and included by **
📒 Files selected for processing (37)
  • .changeset/README.md
  • .changeset/config.json
  • .changeset/sync-tooling.md
  • .commitlintrc.json
  • .eslintignore
  • .eslintrc.json
  • .fixpackrc
  • .github/dependabot.yml
  • .github/workflows/ci.yml
  • .gitignore
  • .husky/.gitignore
  • .husky/commit-msg
  • .husky/pre-commit
  • .lintstagedrc.json
  • .prettierignore
  • .prettierrc.json
  • CHANGELOG.md
  • build.mjs
  • codecov.yml
  • eslint.config.mjs
  • jest-setup.js
  • jest.config.js
  • package.json
  • rolldown.config.mjs
  • src/__tests__/debug.browser.test.ts
  • src/__tests__/debug.server.test.ts
  • src/__tests__/fixtures/legacy-counter/index.js
  • src/__tests__/render.browser.test.ts
  • src/__tests__/render.server.test.ts
  • src/index-browser.ts
  • src/index.ts
  • src/shared.ts
  • src/typings.d.ts
  • src/vitest.d.ts
  • tsconfig.json
  • vitest.config.ts
  • vitest.setup.ts

Walkthrough

This pull request modernizes the project’s tooling and release flow. It adds Changesets and Dependabot configuration, rewrites CI with new triggers, concurrency, matrix testing, and a release job, replaces legacy lint and hook setup with flat ESLint and updated formatting tooling, migrates build and test configuration to Rolldown and Vitest, updates package and TypeScript settings, and applies formatting and test snapshot changes across source files, tests, and the changelog.

Changes

Area Changes
Changesets Added .changeset README, config, and a sync-tooling changeset
CI/CD Added Dependabot and rewrote the GitHub Actions workflow, including release automation
Lint/format Updated Prettier, lint-staged, Husky, and ignore files
Build/Test Added Rolldown and Vitest configs, and updated test matcher typings
Package/TS config Restructured package.json and updated tsconfig.json
Source/tests Reordered imports, reformatted helpers, migrated tests to Vitest APIs, and updated snapshots
Docs Reformatted CHANGELOG.md list styling and spacing

Sequence Diagram(s)

Not applicable.

Related PRs: None identified.
Suggested labels: tooling, ci, dependencies
Suggested reviewers: maintainers familiar with build, test, and release configuration

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main theme: syncing tooling and linting with marko-js/prettier.
Description check ✅ Passed The description is directly related to the changeset and accurately describes the tooling and test modernization.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/marko-testing-library-sync-pwdq86

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/dependabot.yml (1)

7-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider splitting github-actions major vs minor/patch updates.

The npm ecosystem separates major from minor/patch groups (lines 14-20), but the github-actions ecosystem bundles all update types into a single all group. Since major action bumps can be breaking (e.g., actions/checkout v6 changed credential-persistence behavior), grouping them with routine patch/minor bumps risks landing a breaking change unnoticed in a routine-looking PR.

♻️ Suggested grouping
   - package-ecosystem: github-actions
     directory: /
     schedule:
       interval: monthly
     groups:
-      all:
-        patterns: ["*"]
+      minor:
+        patterns: ["*"]
+        update-types: ["minor", "patch"]
+      major:
+        patterns: ["*"]
+        update-types: ["major"]
🤖 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 @.github/dependabot.yml around lines 7 - 9, The github-actions Dependabot
group currently bundles all update types into a single all group, which can mix
breaking major bumps with routine patch/minor updates. Update the github-actions
grouping in dependabot configuration to separate major updates from minor/patch
updates, following the same pattern used by the npm ecosystem, so action
upgrades are reviewed in appropriately scoped PRs.
🤖 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.

Inline comments:
In @.lintstagedrc.json:
- Around line 1-2: The lint-staged config is missing .mjs coverage, so add .mjs
to the existing glob entry in the lint-staged configuration to ensure files like
eslint.config.mjs and build.mjs are included in pre-commit checks. Update the
glob pattern in the JSON config so the same eslint/prettier commands run for
.mjs alongside the current file types.

In `@package.json`:
- Around line 62-79: The test type packages are out of sync with the runtime
dependencies, specifically `@types/jest` and `@types/jsdom` in package.json are
still on older majors while jest and jsdom are newer. Update the versions of
`@types/jest` and `@types/jsdom` to match the current runtime majors, and verify the
existing test setup and any Jest-related typings continue to work with the
updated packages.

---

Nitpick comments:
In @.github/dependabot.yml:
- Around line 7-9: The github-actions Dependabot group currently bundles all
update types into a single all group, which can mix breaking major bumps with
routine patch/minor updates. Update the github-actions grouping in dependabot
configuration to separate major updates from minor/patch updates, following the
same pattern used by the npm ecosystem, so action upgrades are reviewed in
appropriately scoped PRs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2b71b5c7-7ce7-43a6-a69d-0e1d8cfde240

📥 Commits

Reviewing files that changed from the base of the PR and between 7fdb637 and ca1e3d4.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json and included by **
📒 Files selected for processing (29)
  • .changeset/README.md
  • .changeset/config.json
  • .changeset/sync-tooling.md
  • .commitlintrc.json
  • .eslintignore
  • .eslintrc.json
  • .fixpackrc
  • .github/dependabot.yml
  • .github/workflows/ci.yml
  • .gitignore
  • .husky/.gitignore
  • .husky/commit-msg
  • .husky/pre-commit
  • .lintstagedrc.json
  • .prettierignore
  • .prettierrc.json
  • CHANGELOG.md
  • build.mjs
  • eslint.config.mjs
  • package.json
  • src/__tests__/debug.browser.ts
  • src/__tests__/debug.server.ts
  • src/__tests__/render.browser.ts
  • src/__tests__/render.server.ts
  • src/index-browser.ts
  • src/index.ts
  • src/shared.ts
  • src/typings.d.ts
  • tsconfig.json
💤 Files with no reviewable changes (6)
  • .eslintignore
  • .commitlintrc.json
  • .fixpackrc
  • .husky/.gitignore
  • .eslintrc.json
  • .husky/commit-msg

Comment thread .lintstagedrc.json Outdated
Comment thread package.json Outdated
@DylanPiercey DylanPiercey force-pushed the claude/marko-testing-library-sync-pwdq86 branch from ca1e3d4 to c2c2b35 Compare July 3, 2026 19:14
@changeset-bot

changeset-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1592776

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

This PR includes changesets to release 1 package
Name Type
@marko/testing-library 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

@DylanPiercey DylanPiercey force-pushed the claude/marko-testing-library-sync-pwdq86 branch from c2c2b35 to 08adeff Compare July 3, 2026 19:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
package.json (1)

37-51: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

format runs before changeset version in @ci:version, so the generated changelog never gets formatted.

"@ci:version": "npm run build && npm run format && changeset version && npm i --package-lock-only" runs Prettier/ESLint before changeset version writes the new CHANGELOG.md entries and bumps package.json. Those freshly-generated files are never passed through the formatter, so they can fail @ci:lint's prettier . --check on the resulting "Version Packages" PR.

🔧 Suggested fix: format after versioning
-    "`@ci`:version": "npm run build && npm run format && changeset version && npm i --package-lock-only",
+    "`@ci`:version": "npm run build && changeset version && npm run format && npm i --package-lock-only",
🤖 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 `@package.json` around lines 37 - 51, The `@ci`:version script in package.json
formats too early, before changeset version generates the updated changelog and
package metadata. Update the `@ci`:version command so npm run format runs after
changeset version (and after the package-lock update if needed), using the same
script name to locate the change, so the generated CHANGELOG.md and package.json
are included in formatting.
🧹 Nitpick comments (5)
src/shared.ts (1)

160-180: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Consider an explicit origin instead of "*" for the self-postMessage scheduler.

window.postMessage(id, "*") sends to any origin, and the listener at line 164 doesn't check event.origin either. Since this is only used for same-window scheduling, using window.location.origin (with an event.origin check in the listener) would close this off without changing behavior. Impact is limited since only a non-sensitive random id is sent, but this addresses the static analysis finding.

🔒 Optional hardening
           window.addEventListener("message", ({ data }) => {
-            if (data === id) {
+            if (data === id && event.origin === window.location.origin) {
               const callbacks = queue;
               queue = [];
               for (const cb of callbacks) {
                 cb();
               }
             }
           });

           return (cb: Callback) => {
             if (queue.push(cb) === 1) {
-              window.postMessage(id, "*");
+              window.postMessage(id, window.location.origin);
             }
           };
🤖 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 `@src/shared.ts` around lines 160 - 180, The self-postMessage scheduler in
shared.ts currently uses an unrestricted target origin and accepts any origin in
the message handler. Update the anonymous scheduler closure that sets up
window.addEventListener("message", ...) so it verifies event.origin matches
window.location.origin, and change the window.postMessage(id, "*") call to use
the explicit same-origin target instead of "*". Keep the existing queue/id
behavior unchanged.

Source: Linters/SAST tools

.github/workflows/ci.yml (4)

17-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Set persist-credentials: false on checkout for build/test jobs.

Neither job needs to push, so persisting the token in .git/config (default behavior) is unnecessary attack surface (zizmor: artipacked).

🔒 Proposed fix
       - name: Checkout code
         uses: actions/checkout@v6
+        with:
+          persist-credentials: false

(apply to both the build and test checkout steps)

Also applies to: 39-40

🤖 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 @.github/workflows/ci.yml around lines 17 - 18, The Checkout code step in the
build and test jobs is leaving GitHub credentials persisted in `.git/config`,
which is unnecessary for read-only CI. Update the actions/checkout usage in both
jobs to disable credential persistence by setting persist-credentials to false
on the checkout step, keeping the existing checkout behavior otherwise.

Source: Linters/SAST tools


76-76: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider pinning third-party actions to a commit SHA in the release job.

changesets/action@v1 (and actions/checkout@v6 / actions/setup-node@v6 used in this job) are pinned by mutable tag. Given this job runs with contents: write, pull-requests: write, and id-token: write, pinning to an immutable SHA reduces supply-chain risk from a compromised or re-tagged release. The new Dependabot config in this PR can keep SHA-pinned actions up to date automatically.

🤖 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 @.github/workflows/ci.yml at line 76, The release job is using mutable action
tags, so update the workflow to pin the third-party actions to immutable commit
SHAs instead of tags. Adjust the release job entries for changesets/action,
actions/checkout, and actions/setup-node to use SHA references, and keep them
maintainable by relying on Dependabot to update the pinned SHAs automatically.

Source: Linters/SAST tools


14-29: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add explicit least-privilege permissions to build and test jobs.

Neither job declares a permissions block, so they inherit the repository's default token permissions (zizmor: excessive-permissions). Since these jobs only read code, build, lint, and test, an explicit permissions: contents: read scopes the token to the minimum needed, which matters more now that the workflow triggers on pull_request.

🔒 Proposed fix
   build:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     steps:
   test:
     runs-on: ${{ matrix.os }}
+    permissions:
+      contents: read
     name: "test: node ${{ matrix.node }} on ${{ matrix.os }}"

Also applies to: 31-55

🤖 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 @.github/workflows/ci.yml around lines 14 - 29, Add an explicit permissions
block to both the build and test jobs so they do not inherit broad default token
access. In the ci workflow, update the build job and the test job to set
permissions to contents: read only, since the steps in these jobs just checkout,
install, build, lint, and test. Use the existing job names build and test to
locate the right sections and keep all other job behavior unchanged.

Source: Linters/SAST tools


9-11: 🩺 Stability & Availability | 🔵 Trivial

Cancel-in-progress could interrupt an in-flight release.

The concurrency group is keyed on github.head_ref || github.ref, so consecutive pushes to main share a group with cancel-in-progress: true. If a new push lands while release (which depends on build/test) is mid-publish, the in-progress release run could be cancelled partway through, risking a partial version bump/publish. Worth confirming this is an acceptable tradeoff given how infrequently main is pushed to directly.

Also applies to: 56-59

🤖 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 @.github/workflows/ci.yml around lines 9 - 11, The workflow-level concurrency
settings in the CI pipeline are causing release-related runs to share the same
cancelable group as regular main-branch pushes. Update the concurrency logic in
the workflow so the `release` job (and any other publish path) is not cancelled
mid-flight by a later push, while keeping the existing cancellation behavior for
normal build/test runs; use the `concurrency` block and the `release` job
definition to separate these cases.
🤖 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.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 19-24: Add the missing id to the build job’s setup-node step so
the later step name can reference steps.node.outputs.node-version correctly;
update the Install node step in the build job to use the same id: node pattern
already used in the test and release jobs, and keep the Install packages label
interpolation unchanged once the step has a valid id.

---

Duplicate comments:
In `@package.json`:
- Around line 37-51: The `@ci`:version script in package.json formats too early,
before changeset version generates the updated changelog and package metadata.
Update the `@ci`:version command so npm run format runs after changeset version
(and after the package-lock update if needed), using the same script name to
locate the change, so the generated CHANGELOG.md and package.json are included
in formatting.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 17-18: The Checkout code step in the build and test jobs is
leaving GitHub credentials persisted in `.git/config`, which is unnecessary for
read-only CI. Update the actions/checkout usage in both jobs to disable
credential persistence by setting persist-credentials to false on the checkout
step, keeping the existing checkout behavior otherwise.
- Line 76: The release job is using mutable action tags, so update the workflow
to pin the third-party actions to immutable commit SHAs instead of tags. Adjust
the release job entries for changesets/action, actions/checkout, and
actions/setup-node to use SHA references, and keep them maintainable by relying
on Dependabot to update the pinned SHAs automatically.
- Around line 14-29: Add an explicit permissions block to both the build and
test jobs so they do not inherit broad default token access. In the ci workflow,
update the build job and the test job to set permissions to contents: read only,
since the steps in these jobs just checkout, install, build, lint, and test. Use
the existing job names build and test to locate the right sections and keep all
other job behavior unchanged.
- Around line 9-11: The workflow-level concurrency settings in the CI pipeline
are causing release-related runs to share the same cancelable group as regular
main-branch pushes. Update the concurrency logic in the workflow so the
`release` job (and any other publish path) is not cancelled mid-flight by a
later push, while keeping the existing cancellation behavior for normal
build/test runs; use the `concurrency` block and the `release` job definition to
separate these cases.

In `@src/shared.ts`:
- Around line 160-180: The self-postMessage scheduler in shared.ts currently
uses an unrestricted target origin and accepts any origin in the message
handler. Update the anonymous scheduler closure that sets up
window.addEventListener("message", ...) so it verifies event.origin matches
window.location.origin, and change the window.postMessage(id, "*") call to use
the explicit same-origin target instead of "*". Keep the existing queue/id
behavior unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 63e9a768-b673-486b-a08c-cefb7255f8d6

📥 Commits

Reviewing files that changed from the base of the PR and between ca1e3d4 and 08adeff.

⛔ Files ignored due to path filters (4)
  • package-lock.json is excluded by !**/package-lock.json and included by **
  • src/__tests__/__snapshots__/render.browser.test.ts.snap is excluded by !**/*.snap, !**/__snapshots__/** and included by **
  • src/__tests__/__snapshots__/render.server.test.ts.snap is excluded by !**/*.snap, !**/__snapshots__/** and included by **
  • src/__tests__/__snapshots__/render.server.ts.snap is excluded by !**/*.snap, !**/__snapshots__/** and included by **
📒 Files selected for processing (36)
  • .changeset/README.md
  • .changeset/config.json
  • .changeset/sync-tooling.md
  • .commitlintrc.json
  • .eslintignore
  • .eslintrc.json
  • .fixpackrc
  • .github/dependabot.yml
  • .github/workflows/ci.yml
  • .gitignore
  • .husky/.gitignore
  • .husky/commit-msg
  • .husky/pre-commit
  • .lintstagedrc.json
  • .prettierignore
  • .prettierrc.json
  • CHANGELOG.md
  • build.mjs
  • eslint.config.mjs
  • jest-setup.js
  • jest.config.js
  • package.json
  • rolldown.config.mjs
  • src/__tests__/debug.browser.test.ts
  • src/__tests__/debug.server.test.ts
  • src/__tests__/fixtures/legacy-counter/index.js
  • src/__tests__/render.browser.test.ts
  • src/__tests__/render.server.test.ts
  • src/index-browser.ts
  • src/index.ts
  • src/shared.ts
  • src/typings.d.ts
  • src/vitest.d.ts
  • tsconfig.json
  • vitest.config.ts
  • vitest.setup.ts
💤 Files with no reviewable changes (9)
  • .husky/.gitignore
  • .eslintignore
  • jest-setup.js
  • jest.config.js
  • .commitlintrc.json
  • .husky/commit-msg
  • .fixpackrc
  • .eslintrc.json
  • build.mjs
✅ Files skipped from review due to trivial changes (11)
  • .github/dependabot.yml
  • vitest.setup.ts
  • .changeset/README.md
  • .changeset/config.json
  • .lintstagedrc.json
  • src/vitest.d.ts
  • .prettierignore
  • .prettierrc.json
  • .gitignore
  • src/tests/render.browser.test.ts
  • src/typings.d.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • eslint.config.mjs
  • .husky/pre-commit
  • .changeset/sync-tooling.md
  • src/index.ts
  • src/index-browser.ts

Comment thread .github/workflows/ci.yml
@DylanPiercey DylanPiercey force-pushed the claude/marko-testing-library-sync-pwdq86 branch from 08adeff to 9577926 Compare July 3, 2026 19:50

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

14-30: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider explicit least-privilege permissions on build and test jobs.

Neither job declares a permissions block, so they inherit the repo/org default GITHUB_TOKEN scope (which can be broader than contents: read in older repos/orgs). Only release scopes its token explicitly. Adding a minimal permissions: contents: read to build/test is a cheap hardening step flagged by zizmor.

🔧 Proposed fix
   build:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     steps:
   test:
     runs-on: ${{ matrix.os }}
+    permissions:
+      contents: read
     name: "test: node ${{ matrix.node }} on ${{ matrix.os }}"

Also applies to: 31-56

🤖 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 @.github/workflows/ci.yml around lines 14 - 30, Add an explicit minimal
permissions block to the build and test jobs in the CI workflow so they do not
inherit broader default GITHUB_TOKEN scopes. Update the build and test job
definitions in the workflow to set permissions to contents: read, keeping the
existing steps and job names like build and test unchanged.

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.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 9-11: The workflow-level concurrency in ci.yml currently cancels
in-progress runs for both PRs and pushes, which can interrupt release publishing
on main. Update the concurrency settings so cancel-in-progress is only enabled
for pull request runs, while keeping the group key in place; use the
workflow-level concurrency block as the place to apply the conditional logic.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 14-30: Add an explicit minimal permissions block to the build and
test jobs in the CI workflow so they do not inherit broader default GITHUB_TOKEN
scopes. Update the build and test job definitions in the workflow to set
permissions to contents: read, keeping the existing steps and job names like
build and test unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7d6289d2-d407-4471-8f09-d0254db70ad8

📥 Commits

Reviewing files that changed from the base of the PR and between 08adeff and 9577926.

⛔ Files ignored due to path filters (4)
  • package-lock.json is excluded by !**/package-lock.json and included by **
  • src/__tests__/__snapshots__/render.browser.test.ts.snap is excluded by !**/*.snap, !**/__snapshots__/** and included by **
  • src/__tests__/__snapshots__/render.server.test.ts.snap is excluded by !**/*.snap, !**/__snapshots__/** and included by **
  • src/__tests__/__snapshots__/render.server.ts.snap is excluded by !**/*.snap, !**/__snapshots__/** and included by **
📒 Files selected for processing (37)
  • .changeset/README.md
  • .changeset/config.json
  • .changeset/sync-tooling.md
  • .commitlintrc.json
  • .eslintignore
  • .eslintrc.json
  • .fixpackrc
  • .github/dependabot.yml
  • .github/workflows/ci.yml
  • .gitignore
  • .husky/.gitignore
  • .husky/commit-msg
  • .husky/pre-commit
  • .lintstagedrc.json
  • .prettierignore
  • .prettierrc.json
  • CHANGELOG.md
  • build.mjs
  • codecov.yml
  • eslint.config.mjs
  • jest-setup.js
  • jest.config.js
  • package.json
  • rolldown.config.mjs
  • src/__tests__/debug.browser.test.ts
  • src/__tests__/debug.server.test.ts
  • src/__tests__/fixtures/legacy-counter/index.js
  • src/__tests__/render.browser.test.ts
  • src/__tests__/render.server.test.ts
  • src/index-browser.ts
  • src/index.ts
  • src/shared.ts
  • src/typings.d.ts
  • src/vitest.d.ts
  • tsconfig.json
  • vitest.config.ts
  • vitest.setup.ts
💤 Files with no reviewable changes (9)
  • .husky/.gitignore
  • .eslintignore
  • .commitlintrc.json
  • .husky/commit-msg
  • jest.config.js
  • .eslintrc.json
  • .fixpackrc
  • build.mjs
  • jest-setup.js
✅ Files skipped from review due to trivial changes (13)
  • src/vitest.d.ts
  • .husky/pre-commit
  • .lintstagedrc.json
  • rolldown.config.mjs
  • vitest.setup.ts
  • .changeset/README.md
  • .github/dependabot.yml
  • .prettierrc.json
  • .gitignore
  • .prettierignore
  • src/tests/render.browser.test.ts
  • .changeset/sync-tooling.md
  • src/index.ts
🚧 Files skipped from review as they are similar to previous changes (11)
  • .changeset/config.json
  • src/tests/debug.browser.test.ts
  • vitest.config.ts
  • src/typings.d.ts
  • eslint.config.mjs
  • src/tests/fixtures/legacy-counter/index.js
  • src/tests/debug.server.test.ts
  • src/tests/render.server.test.ts
  • src/index-browser.ts
  • tsconfig.json
  • package.json

Comment thread .github/workflows/ci.yml Outdated
Modernize @marko/testing-library's tooling to match marko-js/prettier:

- ESLint flat config (eslint.config.mjs) with typescript-eslint and
  simple-import-sort; drop the legacy .eslintrc/.eslintignore.
- Prettier 3 with prettier-plugin-packagejson.
- Husky 9 and lint-staged (now covering .mjs); remove commitlint/fixpack.
- Changesets for versioning/release in place of standard-version.
- rolldown for bundling (replaces esbuild build.mjs).
- Vitest for testing (replaces jest): server (node) and browser (jsdom)
  projects with .marko compiled per-target via @marko/vite.
- Build/test/release CI workflow, dependabot, and modern tsconfig.
- Upgrade dependencies to latest (Marko 5, jsdom 29,
  @testing-library/jest-dom 6, TypeScript 6, ...); 0 audit vulnerabilities.

Coverage is reported via vitest's istanbul provider; codecov.yml marks the
project/patch status informational since the jest->vitest measurement basis
changes the baseline.
@DylanPiercey DylanPiercey force-pushed the claude/marko-testing-library-sync-pwdq86 branch from 9577926 to 1592776 Compare July 3, 2026 19:58
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.

1 participant