Feat/used homebrew remark plugins#282
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 46d253e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughWorkspace migration to scoped npm packages, auto-readme integration updates, TypeScript modernization of examples and documentation across all packages, schema optionality refinement for LHCI uploads, and robustness improvements for typed-events examples with DOM guards. ChangesScoped packages and example modernization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request migrates several packages to scoped versions of @stephansama/mdast-zone and @stephansama/remark-usage, updates documentation examples to TypeScript using the satisfies operator, and includes example directories in various tsconfig.json files. Feedback highlights a critical runtime error in the lhciUploadSchema where calling .partial() on a discriminated union branch makes the required discriminator optional. Additionally, improvements were suggested for documentation examples to fix broken syntax in code blocks and remove redundant optional chaining after explicit null checks.
| "s/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/UUID/ig", | ||
| ]), | ||
| }) | ||
| .partial(), |
There was a problem hiding this comment.
Calling .partial() on an object that is a member of a z.discriminatedUnion makes the discriminator field (target) optional. Zod requires the discriminator to be a required literal or enum to correctly identify the union branch. This will cause a runtime error during schema initialization.
| .partial(), | |
| .partial().required({ target: true }), |
| first(_payload) { | ||
| ``` | ||
|
|
||
| ```typescript | ||
| }, |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
@stephansama/ai-commit-msg
@stephansama/alfred-kaomoji
@stephansama/astro-iconify-svgmap
@stephansama/auto-readme
@stephansama/catppuccin-jsonresume-theme
@stephansama/catppuccin-opml
@stephansama/catppuccin-rss
@stephansama/catppuccin-typedoc
@stephansama/catppuccin-xsl
@stephansama/eslint-config
create-stephansama-example
@stephansama/find-makefile-targets
@stephansama/github-env
@stephansama/multipublish
@stephansama/pnpm-hooks
@stephansama/prettier-plugin-handlebars
@stephansama/remark-asciinema
@stephansama/single-file
@stephansama/svelte-social-share-links
@stephansama/typed-env
@stephansama/typed-events
@stephansama/typed-nocodb-api
@stephansama/typed-templates
@stephansama/types-github-action-env
@stephansama/types-lhci
commit: |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
core/typed-events/README.md (2)
93-110: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRemove redundant optional chaining after null check.
Line 99 throws if
buttonis null, so the optional chainingbutton?.addEventListeneron line 103 is redundant.🤖 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 `@core/typed-events/README.md` around lines 93 - 110, In dispatchEvent(), you already throw when button is null, so remove the redundant optional chaining on the addEventListener call: replace the button?.addEventListener(...) usage with a direct button.addEventListener(...) call (located in the dispatchEvent function where the click listener is attached) to reflect the guaranteed non-nullness.
76-89:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winInvalid CSS properties
xandyon CSSStyleDeclaration.The properties
style.xandstyle.ydo not exist on the standardCSSStyleDeclarationinterface. This documentation example will cause TypeScript errors and fail at runtime.Consider documenting one of these alternatives:
- CSS transform:
item.style.transform = \translate(${event.data.x}px, ${event.data.y}px)``- CSS custom properties:
item.style.setProperty('--x', String(event.data.x))- Direct element properties for SVG: If this is an SVG element, use
item.setAttribute('x', String(event.data.x))🤖 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 `@core/typed-events/README.md` around lines 76 - 89, The example uses non-existent CSS properties item.style.x and item.style.y which causes TypeScript/runtime errors; update the listenForAnimationEvent example to apply positions via a supported approach (e.g., set item.style.transform = `translate(${event.data.x}px, ${event.data.y}px)` or use item.style.setProperty('--x', String(event.data.x)) / '--y' for CSS variables), or if the element is an SVG use item.setAttribute('x', String(event.data.x)) and item.setAttribute('y', String(event.data.y)) and adjust the querySelector generic (HTMLElement → SVGElement) accordingly; keep the same customAnimationEvent.listen and cleanup handling but replace the invalid style.x/style.y assignments with one of these valid alternatives.
🤖 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 `@core/remark-asciinema/README.md`:
- Line 61: The README imports the module using a repo-internal path ("import
asciinema from \"../dist/index.mjs\"") which won't work for package consumers;
update the example to import from the published package name (e.g., replace that
import with "import asciinema from 'remark-asciinema'" or the actual npm package
name) so the user-facing documentation demonstrates the public API import
instead of a relative dist path.
In `@core/typed-events/example/index.ts`:
- Line 45: The optional chaining on the event hookup is redundant because
`button` is already null-checked and throws earlier; replace
`button?.addEventListener("click", ...)` with a direct call
`button.addEventListener("click", ...)` (locate the `button` variable and the
`addEventListener` call in the example/index.ts and remove the `?`).
In `@core/typed-events/README.md`:
- Around line 310-330: The React example's TypeScript snippet is split into two
separate code blocks which breaks the ExampleComponent function; merge the
fragments into a single ```typescript``` code block that contains the import of
useListeners, the createBroadcastEvent call (map), and the full export function
ExampleComponent with its useListeners({...}) call (including the first and
second listener bodies and closing braces), ensuring the listener function names
(first, second), the createBroadcastEvent, and ExampleComponent are intact and
properly indented so the example can be copy-pasted and compile.
In `@core/types-lhci/src/index.ts`:
- Around line 88-108: The schema for the "lhci" branch is being made fully
optional by calling .partial(), which inadvertently makes the discriminant
target: z.literal("lhci") optional and breaks the z.discriminatedUnion; after
applying .partial() on the object schema, call .extend({ target:
z.literal("lhci") }) (or otherwise re-define target as a required literal) to
re-assert the discriminator requirement so Zod can correctly route to the lhci
branch; apply this change to the schema expression that currently ends with
.partial().
---
Duplicate comments:
In `@core/typed-events/README.md`:
- Around line 93-110: In dispatchEvent(), you already throw when button is null,
so remove the redundant optional chaining on the addEventListener call: replace
the button?.addEventListener(...) usage with a direct
button.addEventListener(...) call (located in the dispatchEvent function where
the click listener is attached) to reflect the guaranteed non-nullness.
- Around line 76-89: The example uses non-existent CSS properties item.style.x
and item.style.y which causes TypeScript/runtime errors; update the
listenForAnimationEvent example to apply positions via a supported approach
(e.g., set item.style.transform = `translate(${event.data.x}px,
${event.data.y}px)` or use item.style.setProperty('--x', String(event.data.x)) /
'--y' for CSS variables), or if the element is an SVG use item.setAttribute('x',
String(event.data.x)) and item.setAttribute('y', String(event.data.y)) and
adjust the querySelector generic (HTMLElement → SVGElement) accordingly; keep
the same customAnimationEvent.listen and cleanup handling but replace the
invalid style.x/style.y assignments with one of these valid alternatives.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e243e682-ac59-4687-9641-99eee4edffaf
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
.config/autoreadmerc.jsoncore/auto-readme/package.jsoncore/auto-readme/src/pipeline.test.tscore/auto-readme/src/pipeline.tscore/auto-readme/src/plugin.tscore/eslint-config/README.mdcore/eslint-config/example/index.tscore/eslint-config/tsconfig.jsoncore/pnpm-hooks/README.mdcore/pnpm-hooks/example/index.jscore/pnpm-hooks/example/index.tscore/pnpm-hooks/tsconfig.jsoncore/prettier-plugin-handlebars/README.mdcore/prettier-plugin-handlebars/example/index.tscore/prettier-plugin-handlebars/tsconfig.jsoncore/remark-asciinema/README.mdcore/remark-asciinema/example/index.tscore/remark-asciinema/tsconfig.jsoncore/single-file/README.mdcore/single-file/example/index.tscore/single-file/tsconfig.jsoncore/typed-env/README.mdcore/typed-env/example/index.tscore/typed-env/tsconfig.jsoncore/typed-events/README.mdcore/typed-events/example/index.tscore/typed-events/tsconfig.jsoncore/typed-nocodb-api/README.mdcore/typed-nocodb-api/example/index.tscore/typed-nocodb-api/tsconfig.jsoncore/typed-templates/README.mdcore/typed-templates/example/index.tscore/typed-templates/tsconfig.jsoncore/types-lhci/README.mdcore/types-lhci/__snapshots__/tsnapi/index.snapshot.d.tscore/types-lhci/example/index.tscore/types-lhci/src/index.tscore/types-lhci/tsconfig.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (3)
- core/pnpm-hooks/example/index.js
- core/typed-env/example/index.ts
- core/typed-templates/example/index.ts
| ```typescript | ||
| import { useListeners } from "../dist/react.mjs"; | ||
|
|
||
| const map = createBroadcastEvent("react-example", { | ||
| first: z.object({}), | ||
| second: z.object({ payload: z.number() }), | ||
| first: z.object({}), | ||
| second: z.object({ payload: z.number() }), | ||
| }); | ||
|
|
||
| export function ExampleComponent() { | ||
| useListeners(map, { | ||
| first: () => console.info("first event happened"), | ||
| second: ({ data }) => console.info(data.payload), | ||
| }); | ||
| useListeners(map, { | ||
| first(_payload) { | ||
| ``` | ||
|
|
||
| ```typescript | ||
| }, | ||
| second(_payload) {}, | ||
| }); | ||
|
|
||
| return; // more jsx... | ||
| return; // more jsx... | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Fix broken code block structure in React example.
The useListeners example is split across two separate TypeScript code blocks (lines 310-321 and 323-330), which breaks the function syntax. This would fail if a reader tries to copy-paste the complete example.
📝 Proposed fix to merge code blocks
-```typescript
-import { useListeners } from "../dist/react.mjs";
-
-const map = createBroadcastEvent("react-example", {
- first: z.object({}),
- second: z.object({ payload: z.number() }),
-});
-
-export function ExampleComponent() {
- useListeners(map, {
- first(_payload) {
-```
-
-```typescript
- },
- second(_payload) {},
- });
-
- return; // more jsx...
-}
-```
+```typescript
+import { useListeners } from "../dist/react.mjs";
+
+const map = createBroadcastEvent("react-example", {
+ first: z.object({}),
+ second: z.object({ payload: z.number() }),
+});
+
+export function ExampleComponent() {
+ useListeners(map, {
+ first(_payload) {
+ //
+ },
+ second(_payload) {},
+ });
+
+ return; // more jsx...
+}
+```🤖 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 `@core/typed-events/README.md` around lines 310 - 330, The React example's
TypeScript snippet is split into two separate code blocks which breaks the
ExampleComponent function; merge the fragments into a single ```typescript```
code block that contains the import of useListeners, the createBroadcastEvent
call (map), and the full export function ExampleComponent with its
useListeners({...}) call (including the first and second listener bodies and
closing braces), ensuring the listener function names (first, second), the
createBroadcastEvent, and ExampleComponent are intact and properly indented so
the example can be copy-pasted and compile.
Checklist
mainhave been mergedmain