-
Notifications
You must be signed in to change notification settings - Fork 138
feat: add hang-ui types #866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis pull request adds TypeScript type declarations for custom HTML elements and reorders a dependency entry. Global type augmentations are added to map the custom elements 'hang-publish-ui' and 'hang-watch-ui' to HTMLElement in their respective TypeScript files. These changes extend the global HTMLElementTagNameMap interface to provide proper type support for the custom elements. A dependency entry in package.json is reordered without functional impact. No runtime logic or exported entities are modified beyond the type declarations. 🚥 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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@js/hang-ui/src/types/global.d.ts`:
- Around line 1-20: global.d.ts is the package "types" entry but does not
reference assets.d.ts which contains ambient module declarations for
`*.css?inline` and `*.svg?raw` used by watch/element.tsx, publish/element.tsx
and shared/components/icon/icon.tsx; add an explicit triple-slash reference to
assets.d.ts at the top of global.d.ts (so TypeScript will include those ambient
module declarations for consumers) and ensure the reference path points to the
assets.d.ts file in the same folder.
In `@js/hang-ui/vite.config.ts`:
- Around line 3-14: The build fails because vite-plugin-dts (imported as dts in
vite.config.ts and used in the plugins array) is not declared as a dependency;
add "vite-plugin-dts" to the hang-ui package's devDependencies (or the workspace
root devDependencies) in js/hang-ui/package.json and run your package manager to
install so the dts plugin is available during vite build.
js/hang-ui/src/types/global.d.ts
Outdated
| declare global { | ||
| namespace JSX { | ||
| interface IntrinsicElements { | ||
| "hang-watch-ui": { | ||
| children?: JSX.Element; | ||
| style?: JSX.CSSProperties; | ||
| class?: string; | ||
| className?: string; | ||
| }; | ||
| "hang-publish-ui": { | ||
| children?: JSX.Element; | ||
| style?: JSX.CSSProperties; | ||
| class?: string; | ||
| className?: string; | ||
| }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find js/hang-ui -type f -name "package.json" | head -5Repository: moq-dev/moq
Length of output: 78
🏁 Script executed:
fd -t f "assets\\.d\\.ts|global\\.d\\.ts|package\\.json" js/hang-ui/Repository: moq-dev/moq
Length of output: 144
🏁 Script executed:
cat js/hang-ui/package.jsonRepository: moq-dev/moq
Length of output: 1257
🏁 Script executed:
cat js/hang-ui/src/types/assets.d.tsRepository: moq-dev/moq
Length of output: 424
🏁 Script executed:
cat -n js/hang-ui/src/types/global.d.tsRepository: moq-dev/moq
Length of output: 545
🏁 Script executed:
rg "assets\.d\.ts|reference.*assets" js/hang-ui/src/ -iRepository: moq-dev/moq
Length of output: 37
🏁 Script executed:
rg "import.*assets|from.*assets" js/hang-ui/src/ -iRepository: moq-dev/moq
Length of output: 37
🌐 Web query:
TypeScript package.json types entry point automatic discovery sibling d.ts files
💡 Result:
TypeScript only needs one “type entry point” for your package (the root import), and then it discovers the rest the same way it discovers JS modules: by following imports/exports and resolving corresponding .d.ts files.
1) What "types" (or "typings") actually does
For import "your-pkg" TypeScript picks a single starting declaration file using this order:
- If
package.jsonhas"types", it uses that path. - Otherwise it uses
"main"and looks for a same-path.d.ts(e.g../dist/index.js→./dist/index.d.ts). - Otherwise it tries
index.d.tsat the package root.
This is summarized in the TS handbook docs. [2]
2) How “sibling” .d.ts files are found automatically
After the entry .d.ts is chosen, TypeScript will only load other .d.ts files if they are reachable through module resolution, e.g.:
- Your entry
index.d.tsre-exports/imports them (export * from "./foo"), or - The consumer imports a subpath that resolves to a
.d.ts(import "your-pkg/foo"), and that subpath is resolvable.
In practice, the easiest “automatic” setup is to publish a parallel tree:
dist/foo.js dist/foo.d.ts
dist/bar/index.js dist/bar/index.d.ts
Then normal resolution can find foo.d.ts, bar/index.d.ts, etc., when those paths are imported. [2]
3) If you use "exports" (recommended), you must also export types for subpaths
With package.json.exports, TypeScript follows the same conditional exports mapping and can use a "types" condition to point at the right .d.ts for each export entry. [1]
If you don’t export a subpath in "exports", consumers generally can’t import it (and TS won’t “discover” its .d.ts) even if the file exists.
4) When you need typesVersions
If you want TypeScript-only remapping (often for older TS versions or alternate folder layouts), use "typesVersions" to redirect your-pkg/* to where the .d.ts files live. [3]
Bottom line: "types" defines the root .d.ts entry; “sibling” .d.ts files are discovered only if they’re resolvable via imports/subpath resolution (and with "exports", you must export those subpaths and ideally provide "types" conditions). [1][2][3]
🏁 Script executed:
cat js/hang-ui/src/publish/index.d.tsRepository: moq-dev/moq
Length of output: 120
🏁 Script executed:
cat js/hang-ui/src/watch/index.d.tsRepository: moq-dev/moq
Length of output: 118
🏁 Script executed:
rg "\.css\?inline|\.svg\?raw|worker&url" js/hang-ui/src/ --type tsx --type tsRepository: moq-dev/moq
Length of output: 82
🏁 Script executed:
fd . js/hang-ui/src/publish/ js/hang-ui/src/watch/ -t f | head -20Repository: moq-dev/moq
Length of output: 1054
🏁 Script executed:
rg "css\?inline|svg\?raw|worker&url" js/hang-ui/src/ --type-listRepository: moq-dev/moq
Length of output: 444
🏁 Script executed:
rg "\.css\?inline|\.svg\?raw|worker&url" js/hang-ui/src/Repository: moq-dev/moq
Length of output: 2189
🏁 Script executed:
fd vite.config -t f js/hang-ui/Repository: moq-dev/moq
Length of output: 80
🏁 Script executed:
cat js/hang-ui/vite.config.ts 2>/dev/null || cat js/hang-ui/vite.config.js 2>/dev/nullRepository: moq-dev/moq
Length of output: 699
Add explicit reference to assets.d.ts in the package type entry point.
The assets.d.ts file contains ambient module declarations for *.css?inline and *.svg?raw imports used throughout the source code (watch/element.tsx, publish/element.tsx, shared/components/icon/icon.tsx). Since package.json "types" points only to global.d.ts, TypeScript will not automatically discover assets.d.ts. Add an explicit reference to ensure consumers see these type definitions.
🔧 Suggested fix
+/// <reference path="./assets.d.ts" />
+
declare global {
namespace JSX {
interface IntrinsicElements {
"hang-watch-ui": {📝 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.
| declare global { | |
| namespace JSX { | |
| interface IntrinsicElements { | |
| "hang-watch-ui": { | |
| children?: JSX.Element; | |
| style?: JSX.CSSProperties; | |
| class?: string; | |
| className?: string; | |
| }; | |
| "hang-publish-ui": { | |
| children?: JSX.Element; | |
| style?: JSX.CSSProperties; | |
| class?: string; | |
| className?: string; | |
| }; | |
| } | |
| } | |
| } | |
| export {}; | |
| /// <reference path="./assets.d.ts" /> | |
| declare global { | |
| namespace JSX { | |
| interface IntrinsicElements { | |
| "hang-watch-ui": { | |
| children?: JSX.Element; | |
| style?: JSX.CSSProperties; | |
| class?: string; | |
| className?: string; | |
| }; | |
| "hang-publish-ui": { | |
| children?: JSX.Element; | |
| style?: JSX.CSSProperties; | |
| class?: string; | |
| className?: string; | |
| }; | |
| } | |
| } | |
| } | |
| export {}; |
🤖 Prompt for AI Agents
In `@js/hang-ui/src/types/global.d.ts` around lines 1 - 20, global.d.ts is the
package "types" entry but does not reference assets.d.ts which contains ambient
module declarations for `*.css?inline` and `*.svg?raw` used by
watch/element.tsx, publish/element.tsx and shared/components/icon/icon.tsx; add
an explicit triple-slash reference to assets.d.ts at the top of global.d.ts (so
TypeScript will include those ambient module declarations for consumers) and
ensure the reference path points to the assets.d.ts file in the same folder.
js/hang-ui/vite.config.ts
Outdated
| import dts from "vite-plugin-dts"; | ||
| import solidPlugin from "vite-plugin-solid"; | ||
|
|
||
| export default defineConfig({ | ||
| plugins: [solidPlugin()], | ||
| plugins: [ | ||
| solidPlugin(), | ||
| dts({ | ||
| include: ["src"], | ||
| exclude: ["src/vite-env.d.ts"], | ||
| entryRoot: "src", | ||
| copyDtsFiles: true, | ||
| }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "vite-plugin-dts" -g "package.json"Repository: moq-dev/moq
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Find and examine package.json files in js/hang-ui and root
echo "=== js/hang-ui/package.json ==="
cat js/hang-ui/package.json 2>/dev/null || echo "File not found"
echo -e "\n=== Root package.json ==="
cat package.json 2>/dev/null | head -50 || echo "File not found"
echo -e "\n=== Workspace structure check ==="
fd -e "package.json" -t f | head -20Repository: moq-dev/moq
Length of output: 1740
Declare vite-plugin-dts as a devDependency for hang-ui.
The config imports and uses vite-plugin-dts, but it isn't listed in js/hang-ui/package.json or the root workspace package.json. This will cause the build to fail when running vite build.
🤖 Prompt for AI Agents
In `@js/hang-ui/vite.config.ts` around lines 3 - 14, The build fails because
vite-plugin-dts (imported as dts in vite.config.ts and used in the plugins
array) is not declared as a dependency; add "vite-plugin-dts" to the hang-ui
package's devDependencies (or the workspace root devDependencies) in
js/hang-ui/package.json and run your package manager to install so the dts
plugin is available during vite build.
js/hang-ui/src/types/global.d.ts
Outdated
| @@ -0,0 +1,20 @@ | |||
| declare global { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afaik this is generally the "correct" way of registering WebComponents:
moq/js/hang/src/watch/element.ts
Line 215 in 78bc805
| "hang-watch": HangWatch; |
As a bonus, I don't think any of this file is needed? It'll get rolled into the exported .d.ts. It's less prone to getting out of sync too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! I will move the type declarations to the component files using HTMLElementTagNameMap as you suggested.
Something like this:
declare global {
interface HTMLElementTagNameMap {
"hang-watch-ui": HTMLElement;
}
}
We don't have custom props, so HTMLElement should be enough. What do you think?
I'll also remove the unnecessary files. Thank you!
js/hang-ui/package.json
Outdated
| "description": "Media over QUIC library UI components", | ||
| "license": "(MIT OR Apache-2.0)", | ||
| "repository": "github:moq-dev/moq", | ||
| "types": "./src/types/global.d.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is wrong because we have separate exports.
Provides TypeScript types for Hang UI.