Conversation
vite-7/src/components/HangWatch.tsx
Outdated
| import '@moq/hang-ui/watch'; | ||
| import '@moq/hang/watch/element'; | ||
|
|
||
| interface HangWatchElement extends HTMLElement { |
There was a problem hiding this comment.
Is there a way to get this automatically? I don't want users manually creating types.
There was a problem hiding this comment.
I agree with you. I just opened a new PR with a specific declaration and some other fixes (I tested locally using npm pack). I tried a few different methods, but for React, separate declaration files seem to be the only solution so far. Feel free to suggest another way if you have any other ideas.
There was a problem hiding this comment.
Sorry it took me some time to get back to you. I’ve introduced CEM (Custom Elements Manifest) to manage the different components. I believe this is a good solution to avoid type issues, generate documentation automatically, and easily create wrappers for different frameworks.
We will create wrappers for all the frameworks we want to use. As an example, in this PR, I added the React wrappers for @moq/hang and @moq/hang-ui. What do you think?
Here is an example of how the components look in React:
import { type FC } from 'react';
import '@moq/hang-ui/watch';
import '@moq/hang/watch/element';
import { HangWatchUI } from '@moq/hang-ui/react';
import { HangWatch } from '@moq/hang/react';
const HangWatchComponent: FC<{ url: string; path: string }> = ({ url, path }) => {
return (
<HangWatchUI>
<HangWatch url={url} path={path} jitter={100} muted reload volume={0}>
<canvas style={{ width: '100%', height: 'auto' }} width="1280" height="720"></canvas>
</HangWatch>
</HangWatchUI>
);
};
export default HangWatchComponent;| element.setAttribute('url', url); | ||
| element.setAttribute('path', path); |
There was a problem hiding this comment.
We should encourage using the type-safe properties, not attributes.
There was a problem hiding this comment.
I totally agree. I added the setters in @moq/hang specifically for this reason. This way, React and other frameworks can use type-safe properties directly via props instead of attributes.
vite-7/src/components/HangWatch.tsx
Outdated
| element.setAttribute('url', url); | ||
| element.setAttribute('path', path); | ||
| element.setAttribute('muted', ''); | ||
| element.setAttribute('latency', '100'); | ||
| element.setAttribute('reload', ''); |
There was a problem hiding this comment.
Ditto, element.url should work.
There was a problem hiding this comment.
I added the setter for the url property here as well, so we don't need to use the attribute.
| import react from '@vitejs/plugin-react' | ||
|
|
||
| // Rewrite Vite-only worklet imports inside @moq packages during transform | ||
| function moqWorkletTransform() { |
There was a problem hiding this comment.
:/ Why is this needed?
We need to fix this if users actually have to do this.
There was a problem hiding this comment.
I totally agree! Unfortunately, if I remove this plugin, an error appears and the Vite dev server stops working. It seems like a compilation problem with .ts?worker&url. I'm not sure why it happens yet.
| export default defineConfig({ | ||
| plugins: [react(), moqWorkletTransform()], | ||
| optimizeDeps: { | ||
| exclude: ["@moq/hang", "@moq/hang-ui"], |
There was a problem hiding this comment.
We should figure out why this is needed
There was a problem hiding this comment.
I totally agree, I will investigate this.
There was a problem hiding this comment.
After introducing CEM, several issues were automatically resolved.
Currently, in the vite-7 folder (which I'm using for testing with npm pack), I still have this exclusion:
optimizeDeps: {
exclude: ["@moq/hang"]
}If I remove the optimizeDeps exclusion, I receive the following error:
I will look into this later.
There was a problem hiding this comment.
I don't think this is needed any longer since the switch to moq/watch and moq/publish
NOTE: Requires moq/watch/publish 0.2.0 to be published.
WalkthroughThis pull request introduces a new 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
vite-7/vite.config.ts (1)
5-20: Consider typing the plugin return value for better IDE support.The function returns a plain object literal. Typing it as
Pluginfrom Vite would provide autocomplete and catch typos in hook names.Proposed fix
-function moqWorkletTransform() { - return { +import { defineConfig, type Plugin } from 'vite' + +function moqWorkletTransform(): Plugin { + return {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite-7/vite.config.ts` around lines 5 - 20, Add Vite plugin typing by importing the Plugin type from 'vite' and annotate the moqWorkletTransform return type as Plugin (e.g., change the function signature to return Plugin or convert it to a typed const). Update the signature of moqWorkletTransform and keep the existing object shape (name, enforce, transform) so the returned object conforms to Plugin, which will enable IDE autocomplete and catch hook-name typos in the transform implementation that references code and id.vite-7/src/index.css (1)
1-14: Fix Stylelint violations in:rootblock.The static analysis tool flagged three issues in this block:
- Lines 6 and 10: unexpected empty lines before declarations (
declaration-empty-line-before).- Line 11:
optimizeLegibilityshould beoptimizelegibility(value-keyword-case).These come from the default Vite scaffold template. If Stylelint is enforced in CI, they'll need fixing:
Proposed fix
:root { font-family: Inter, system-ui, Avenir, Helvetica, Arial, sans-serif; line-height: 1.5; font-weight: 400; - color-scheme: light dark; color: rgba(255, 255, 255, 0.87); background-color: `#242424`; - font-synthesis: none; - text-rendering: optimizeLegibility; + text-rendering: optimizelegibility; -webkit-font-smoothing: antialiased; -moz-osx-font-smoothing: grayscale; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite-7/src/index.css` around lines 1 - 14, Remove the unexpected empty lines in the :root block (fix the declaration-empty-line-before violations) so declarations are contiguous, and change the text-rendering value from optimizeLegibility to the lowercase optimizelegibility to satisfy value-keyword-case; specifically edit the :root selector (properties like font-weight, color-scheme, color, background-color, font-synthesis, text-rendering, -webkit-font-smoothing, -moz-osx-font-smoothing) to remove blank lines before declarations and use the lowercase keyword for text-rendering.vite-7/src/App.css (1)
1-42: Most of these styles appear unused.
App.tsxonly rendersmoq-watchandmoq-publishcustom elements. The.logo,.card,.read-the-docs, andlogo-spinanimation classes from the Vite scaffold template don't seem to be referenced anywhere. Consider trimming this down to only the styles actually needed (e.g.,#root).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite-7/src/App.css` around lines 1 - 42, Remove unused scaffold styles and keep only what's needed for the actual app layout: trim out .logo, .logo.react, .logo:hover, `@keyframes` logo-spin, the media rule targeting a:nth-of-type(2) .logo, .card, and .read-the-docs from App.css, leaving the `#root` rules (and any other rules referenced by moq-watch or moq-publish if present). Verify App.tsx (which renders moq-watch and moq-publish) does not rely on any of the removed selectors before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vite-7/package.json`:
- Around line 13-14: The package version ranges for `@moq/publish` and `@moq/watch`
are currently "^0.1.1" which excludes the required 0.2.0 release; update the
dependency entries for "@moq/publish" and "@moq/watch" in package.json to
reference a range that includes 0.2.0 (e.g. change "^0.1.1" to "^0.2.0" or
">=0.2.0 <0.3.0") so the published 0.2.0 can be installed.
In `@vite-7/README.md`:
- Around line 44-52: The README.md project-structure fenced code block lacks a
language specifier causing MD040; update the triple-backtick fence that
surrounds the tree (the block starting with "src/") to include a language tag
like text or plaintext (e.g., ```text) so markdownlint stops warning, keeping
the block content unchanged.
- Around line 18-21: Update the README command examples so they use npm run for
custom scripts: replace occurrences of "npm dev" with "npm run dev" and "npm
build" with "npm run build" (leave "npm install" as-is); ensure references to
the scripts match the package.json script names (e.g., dev, build) so users run
the correct commands.
- Line 3: Update the README text to use the correct package names: replace
references to `@moq/hang` and `@moq/hang-ui` with the actual runtime
dependencies `@moq/publish` and `@moq/watch` (both in the initial description
and in the Resources section), ensuring any example commands or links reflect
`@moq/publish` and `@moq/watch` instead of the old names so documentation
matches package.json.
In `@vite-7/src/App.tsx`:
- Around line 14-16: The hardcoded JWT in the moq-watch element's url attribute
is a secret risk; remove the literal token and either (preferred) load the full
URL from an environment variable (e.g. use Vite's import.meta.env.VITE_MOQ_URL
or process.env.VITE_MOQ_URL) when rendering the <moq-watch> component, or (if it
must remain) add a clear inline comment next to the <moq-watch url=...>
attribute stating this is a public demo non-expiring token and should not be
used in production; update any code that constructs the url so it reads from the
env variable and falls back to a documented demo URL only in dev.
---
Nitpick comments:
In `@vite-7/src/App.css`:
- Around line 1-42: Remove unused scaffold styles and keep only what's needed
for the actual app layout: trim out .logo, .logo.react, .logo:hover, `@keyframes`
logo-spin, the media rule targeting a:nth-of-type(2) .logo, .card, and
.read-the-docs from App.css, leaving the `#root` rules (and any other rules
referenced by moq-watch or moq-publish if present). Verify App.tsx (which
renders moq-watch and moq-publish) does not rely on any of the removed selectors
before committing.
In `@vite-7/src/index.css`:
- Around line 1-14: Remove the unexpected empty lines in the :root block (fix
the declaration-empty-line-before violations) so declarations are contiguous,
and change the text-rendering value from optimizeLegibility to the lowercase
optimizelegibility to satisfy value-keyword-case; specifically edit the :root
selector (properties like font-weight, color-scheme, color, background-color,
font-synthesis, text-rendering, -webkit-font-smoothing, -moz-osx-font-smoothing)
to remove blank lines before declarations and use the lowercase keyword for
text-rendering.
In `@vite-7/vite.config.ts`:
- Around line 5-20: Add Vite plugin typing by importing the Plugin type from
'vite' and annotate the moqWorkletTransform return type as Plugin (e.g., change
the function signature to return Plugin or convert it to a typed const). Update
the signature of moqWorkletTransform and keep the existing object shape (name,
enforce, transform) so the returned object conforms to Plugin, which will enable
IDE autocomplete and catch hook-name typos in the transform implementation that
references code and id.
| "@moq/publish": "^0.1.1", | ||
| "@moq/watch": "^0.1.1", |
There was a problem hiding this comment.
^0.1.1 semver range won't include the required 0.2.0 release.
For 0.x packages, ^0.1.1 resolves to >=0.1.1 <0.2.0. Per the commit notes, this example depends on moq/watch/publish 0.2.0, which falls outside that range. Once 0.2.0 is published, the version constraints here need to be bumped so users can actually install the compatible version.
📦 Proposed fix
- "@moq/publish": "^0.1.1",
- "@moq/watch": "^0.1.1",
+ "@moq/publish": "^0.2.0",
+ "@moq/watch": "^0.2.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@moq/publish": "^0.1.1", | |
| "@moq/watch": "^0.1.1", | |
| "@moq/publish": "^0.2.0", | |
| "@moq/watch": "^0.2.0", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite-7/package.json` around lines 13 - 14, The package version ranges for
`@moq/publish` and `@moq/watch` are currently "^0.1.1" which excludes the required
0.2.0 release; update the dependency entries for "@moq/publish" and "@moq/watch"
in package.json to reference a range that includes 0.2.0 (e.g. change "^0.1.1"
to "^0.2.0" or ">=0.2.0 <0.3.0") so the published 0.2.0 can be installed.
| @@ -0,0 +1,59 @@ | |||
| # React 19 + TypeScript + Vite 7 | |||
|
|
|||
| Minimal integration example demonstrating `@moq/hang` and `@moq/hang-ui` with React and Vite. | |||
There was a problem hiding this comment.
Package names in the description don't match package.json.
Line 3 describes demonstrating @moq/hang and @moq/hang-ui, but the actual runtime dependencies are @moq/publish and @moq/watch. The Resources section at lines 56–57 has the same mismatch. This looks like the README was drafted against an earlier API surface.
✏️ Proposed fix
-Minimal integration example demonstrating `@moq/hang` and `@moq/hang-ui` with React and Vite.
+Minimal integration example demonstrating `@moq/publish` and `@moq/watch` with React and Vite.And in the Resources section:
-- [`@moq/hang`](https://www.npmjs.com/package/@moq/hang)
-- [`@moq/hang-ui`](https://www.npmjs.com/package/@moq/hang-ui)
++ [`@moq/publish`](https://www.npmjs.com/package/@moq/publish)
++ [`@moq/watch`](https://www.npmjs.com/package/@moq/watch)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Minimal integration example demonstrating `@moq/hang` and `@moq/hang-ui` with React and Vite. | |
| Minimal integration example demonstrating `@moq/publish` and `@moq/watch` with React and Vite. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite-7/README.md` at line 3, Update the README text to use the correct
package names: replace references to `@moq/hang` and `@moq/hang-ui` with the
actual runtime dependencies `@moq/publish` and `@moq/watch` (both in the initial
description and in the Resources section), ensuring any example commands or
links reflect `@moq/publish` and `@moq/watch` instead of the old names so
documentation matches package.json.
| ```bash | ||
| npm install | ||
| npm dev | ||
| ``` |
There was a problem hiding this comment.
npm dev and npm build are not valid npm commands — use npm run.
npm dev and npm build will fail because dev and build are custom scripts, not npm lifecycle commands.
✏️ Proposed fix
-npm install
-npm dev
+npm install
+npm run dev-npm build
+npm run build📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```bash | |
| npm install | |
| npm dev | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite-7/README.md` around lines 18 - 21, Update the README command examples so
they use npm run for custom scripts: replace occurrences of "npm dev" with "npm
run dev" and "npm build" with "npm run build" (leave "npm install" as-is);
ensure references to the scripts match the package.json script names (e.g., dev,
build) so users run the correct commands.
| ``` | ||
| src/ | ||
| ├── App.tsx # Main component with mode toggle | ||
| ├── components/ | ||
| │ ├── HangWatch.tsx # Stream viewer | ||
| │ └── HangPublish.tsx # Stream publisher | ||
| ├── App.css # Component styles | ||
| └── index.css # Global styles | ||
| ``` |
There was a problem hiding this comment.
Missing language specifier on fenced code block (MD040).
The project-structure block has no language tag, triggering a markdownlint warning. Use text (or plaintext) to suppress it.
✏️ Proposed fix
-```
+```text
src/
├── App.tsx # Main component with mode toggle📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| src/ | |
| ├── App.tsx # Main component with mode toggle | |
| ├── components/ | |
| │ ├── HangWatch.tsx # Stream viewer | |
| │ └── HangPublish.tsx # Stream publisher | |
| ├── App.css # Component styles | |
| └── index.css # Global styles | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite-7/README.md` around lines 44 - 52, The README.md project-structure
fenced code block lacks a language specifier causing MD040; update the
triple-backtick fence that surrounds the tree (the block starting with "src/")
to include a language tag like text or plaintext (e.g., ```text) so markdownlint
stops warning, keeping the block content unchanged.
| <moq-watch | ||
| url="https://cdn.moq.dev/demo?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJyb290IjoiZGVtbyIsImdldCI6WyIiXSwiZXhwIjpudWxsLCJpYXQiOm51bGx9.6EoN-Y1Ouj35_qV5FokcdcdderrE2navNbYQjJyR2Ac" | ||
| path="bbb" |
There was a problem hiding this comment.
Hardcoded JWT token in source code.
Gitleaks flagged this JWT. Decoding it shows {"root":"demo","get":[""],"exp":null,"iat":null} — a non-expiring, read-only demo token for cdn.moq.dev. While this is likely intentional for a public example, a non-expiring token is a risk if the signing secret is ever compromised.
Consider either:
- Adding a comment documenting that this is a public demo token, or
- Loading the URL from an environment variable so it's not baked into source
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 15-15: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite-7/src/App.tsx` around lines 14 - 16, The hardcoded JWT in the moq-watch
element's url attribute is a secret risk; remove the literal token and either
(preferred) load the full URL from an environment variable (e.g. use Vite's
import.meta.env.VITE_MOQ_URL or process.env.VITE_MOQ_URL) when rendering the
<moq-watch> component, or (if it must remain) add a clear inline comment next to
the <moq-watch url=...> attribute stating this is a public demo non-expiring
token and should not be used in production; update any code that constructs the
url so it reads from the env variable and falls back to a documented demo URL
only in dev.
2 participants
I added a new example folder:
vite-7.This folder shows how to use
@moq/hangand@moq/hang-uiwith React 19 and Vite 7.It includes a simple project setup and sample code for publishing and watching streams.