-
-
Notifications
You must be signed in to change notification settings - Fork 145
feat/claude skills #786
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
feat/claude skills #786
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR introduces a new CLI command Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (35)
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 |
| .toISOString() | ||
| .replace(/[:.]/g, '') | ||
| .replace('T', '') | ||
| .replace('Z', 'Z'); |
Check warning
Code scanning / CodeQL
Replacement of a substring with itself Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
In general, to fix a "replacement of a substring with itself" issue, either remove the redundant replacement if no transformation is needed, or correct the replacement string to reflect the intended change (e.g., escaping or substituting with another character). The goal is to avoid confusing no-op calls that suggest behavior that isn’t actually happening.
Here, backupFileSync builds timestamp using:
const timestamp = new Date()
.toISOString()
.replace(/[:.]/g, '')
.replace('T', '')
.replace('Z', 'Z');The last .replace('Z', 'Z') is a no-op. The earlier transformations already make the string safe and compact (2025-01-01T12:34:56.789Z → 20250101T123456789Z → 20250101123456789Z). If the only goal is a compact, safe timestamp, the cleanest and least risky fix is simply to remove the final .replace('Z', 'Z') call. This preserves current output (apart from not doing an unnecessary operation) and avoids implying there is any special handling of Z. Concretely, in cli/src/agents.ts, modify the timestamp construction in backupFileSync so that it ends after .replace('T', ''), deleting the .replace('Z', 'Z') line.
No new imports or helper methods are needed; this is just a minor edit to the method chain.
-
Copy modified line R40
| @@ -37,8 +37,7 @@ | ||
| const timestamp = new Date() | ||
| .toISOString() | ||
| .replace(/[:.]/g, '') | ||
| .replace('T', '') | ||
| .replace('Z', 'Z'); | ||
| .replace('T', ''); | ||
| const backupPath = `${filePath}.bak.${timestamp}`; | ||
| fs.copyFileSync(filePath, backupPath); | ||
| return backupPath; |
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: 6
Fix all issues with AI Agents 🤖
In @cli/package.json:
- Line 42: The dependency entry "@openwebf/claude-code-skills": "^1.0.1" in
package.json is invalid (not on npm); remove that dependency line or replace it
with a valid published package/version (or move it to a private/internal
registry configuration) so installs won’t fail; update any code importing the
symbol (e.g., imports referencing @openwebf/claude-code-skills) to use the new
package name/version or guard/remove those imports accordingly.
In @skills/PUBLISHING.md:
- Line 26: The example uses a hardcoded absolute path "cd
/Users/andycall/workspace/webf/skills"; replace it with a generic relative path
or placeholder (e.g., "cd ./skills" or "cd <path-to-repo>/skills") so the
command works on other machines and clearly indicates it should be adapted to
the reader's environment.
In @skills/README.md:
- Around line 547-557: The README currently hardcodes an absolute path
"/Users/andycall/workspace/webf/skills/"; replace it with a relative or generic
placeholder (e.g., "./skills/" or "<repo-root>/skills/") in the "Skill Files
Location" section and update the surrounding text so the list entries
(webf-quickstart/, webf-async-rendering/, etc.) remain the same; ensure no other
absolute local paths remain in that section and keep the phrasing generic for
any contributor.
In @skills/webf-async-rendering/SKILL.md:
- Around line 300-305: Replace the bare URLs under the "Resources" section with
proper Markdown link syntax: turn each bullet that currently shows a plain URL
into a bracketed link label followed by the URL in parentheses (e.g., for "Core
Concepts - Async Rendering", "Debugging & Performance", and the
"@openwebf/react-core-ui" install note) so all three entries are formatted as
Markdown links rather than raw URLs.
In @skills/webf-native-plugin-dev/SKILL.md:
- Line 1421: Replace the hardcoded absolute path
"/Users/andycall/workspace/webf/native_plugins/share" in SKILL.md with a
repository-relative or generic placeholder path (e.g., "./native_plugins/share"
or "<project-root>/native_plugins/share") so other users can locate the
implementation; update the sentence "See the complete implementation at
`/Users/andycall/workspace/webf/native_plugins/share` for:" to use the
relative/path placeholder instead and ensure any README or docs reference the
repository root or an environment variable rather than a user-specific absolute
path.
In @skills/webf-quickstart/reference.md:
- Around line 89-109: The docs currently recommend installing
@vitejs/plugin-basic-ssl and enabling server.https with basicSsl(), but you must
clarify that the plugin generates self-signed, untrusted certificates (browser
warnings) and that Vite recommends supplying your own trusted certs via the
server.https option; update the vite.config.js section to note the plugin is OK
for quick local testing but advise users who need trusted HTTPS to
generate/provide certificate and key files and configure server.https
accordingly (or link to Vite’s docs for server.https and steps to create trusted
certs), and explicitly state whether the plugin approach is acceptable for WebF
mobile testing vs when to use real certs.
🧹 Nitpick comments (40)
skills/webf-native-ui/reference.md (1)
548-554: Wrap bare URLs in markdown link syntax.The Resources section contains bare URLs that should be wrapped in markdown link format for better markdown compliance and clickability.
🔎 Proposed fix
## Resources -- **Official Documentation**: https://openwebf.com/en/ui-components -- **Cupertino Gallery**: https://openwebf.com/en/ui-components/cupertino -- **SF Symbols Browser**: https://developer.apple.com/sf-symbols/ -- **Form Validation**: https://pub.dev/packages/form_builder_validators +- **Official Documentation**: [https://openwebf.com/en/ui-components](https://openwebf.com/en/ui-components) +- **Cupertino Gallery**: [https://openwebf.com/en/ui-components/cupertino](https://openwebf.com/en/ui-components/cupertino) +- **SF Symbols Browser**: [https://developer.apple.com/sf-symbols/](https://developer.apple.com/sf-symbols/) +- **Form Validation**: [https://pub.dev/packages/form_builder_validators](https://pub.dev/packages/form_builder_validators)skills/webf-native-ui/SKILL.md (1)
322-328: Wrap bare URLs in markdown link syntax.The Resources section contains bare URLs that should be wrapped in markdown link format for consistency with markdown best practices.
🔎 Proposed fix
## Resources -- **Component Gallery**: https://openwebf.com/en/ui-components -- **Cupertino UI Docs**: https://openwebf.com/en/ui-components/cupertino -- **WebF CLI Docs**: https://openwebf.com/en/docs/tools/webf-cli -- **React Examples**: https://github.com/openwebf/react-cupertino-gallery -- **Vue Examples**: https://github.com/openwebf/vue-cupertino-gallery +- **Component Gallery**: [https://openwebf.com/en/ui-components](https://openwebf.com/en/ui-components) +- **Cupertino UI Docs**: [https://openwebf.com/en/ui-components/cupertino](https://openwebf.com/en/ui-components/cupertino) +- **WebF CLI Docs**: [https://openwebf.com/en/docs/tools/webf-cli](https://openwebf.com/en/docs/tools/webf-cli) +- **React Examples**: [https://github.com/openwebf/react-cupertino-gallery](https://github.com/openwebf/react-cupertino-gallery) +- **Vue Examples**: [https://github.com/openwebf/vue-cupertino-gallery](https://github.com/openwebf/vue-cupertino-gallery)skills/webf-api-compatibility/SKILL.md (3)
256-256: Wrap bare URLs in markdown link syntax.Lines containing bare URLs should use
[text](url)format for consistency and to pass markdown linting:
- Line 256:
https://openwebf.com/en/native-plugins- Line 260:
https://openwebf.com/en/native-plugins- Line 331:
https://openwebf.com/en/native-plugins- Lines 436-439: Multiple bare URLs
🔎 Example fix for bare URLs
-Check available plugins: https://openwebf.com/en/native-plugins +Check available plugins: [https://openwebf.com/en/native-plugins](https://openwebf.com/en/native-plugins)Or with descriptive text:
-https://openwebf.com/en/docs/developer-guide/core-concepts#api-compatibility +[API Compatibility Table](https://openwebf.com/en/docs/developer-guide/core-concepts#api-compatibility)Also applies to: 260-260, 331-331, 436-439
270-273: Convert emphasis-styled subheadings to proper markdown headings.Instances like
**If using WebF Go:**use emphasis instead of heading markup. Use### Headingsyntax for better document structure and accessibility:🔎 Example fix for emphasis headings
-**If using WebF Go:** +### Option 1: Testing in WebF Go-**Option 1: Testing in WebF Go** (Most web developers) +### Option 1: Testing in WebF GoAlso applies to: 282-286
444-455: Specify language for the decision tree code block.The fenced code block starting at line 444 (the decision tree) lacks a language identifier. Use
```textor```markdownfor clarity:🔎 Example fix
-``` +```text Need storage? ├─ Simple key-value → localStorage ✅skills/webf-api-compatibility/alternatives.md (2)
179-179: Wrap bare URLs in markdown link syntax.This file has multiple bare URLs that should use
[text](url)format:
- Line 179:
https://openwebf.com/en/native-plugins- Line 185:
https://openwebf.com/en/native-plugins- Line 214:
https://openwebf.com/en/native-plugins/webf-share- Line 331:
https://openwebf.com/en/native-plugins- Line 417:
https://openwebf.com/en/docs/add-webf-to-flutter/bridge-modules- Lines 514-518: Multiple URLs in Resources section
🔎 Example for Resources section fix
-**Official Plugins List**: https://openwebf.com/en/native-plugins (Check here first!) -**WebF Plugins on npm**: https://www.npmjs.com/search?q=%40openwebf +**Official Plugins List**: [https://openwebf.com/en/native-plugins](https://openwebf.com/en/native-plugins) (Check here first!) +**WebF Plugins on npm**: [https://www.npmjs.com/search?q=%40openwebf](https://www.npmjs.com/search?q=%40openwebf)Also applies to: 185-185, 214-214, 331-331, 417-417, 514-518
195-205: Convert emphasis-styled subheadings to proper markdown headings.Similar to SKILL.md, instances like
**Option 1: Testing in WebF Go**use emphasis instead of heading markup:🔎 Example fix
-**Option 1: Testing in WebF Go** (Most web developers) -✅ Just install npm package -✅ No additional setup needed -✅ WebF Go already has Flutter plugins included +### Option 1: Testing in WebF Go + +(Most web developers) +- ✅ Just install npm package +- ✅ No additional setup needed +- ✅ WebF Go already has Flutter plugins includedAlso applies to: 229-240
skills/webf-api-compatibility/reference.md (1)
379-381: Wrap bare URLs in markdown link syntax in Resources section.The final Resources section uses bare URLs:
🔎 Example fix
-- **Full Documentation**: https://openwebf.com/en/docs -- **API Reference**: https://openwebf.com/en/docs/api -- **GitHub Discussions**: Ask compatibility questions -- **Native Plugins**: See `alternatives.md` in this skill +- **Full Documentation**: [https://openwebf.com/en/docs](https://openwebf.com/en/docs) +- **API Reference**: [https://openwebf.com/en/docs/api](https://openwebf.com/en/docs/api) +- **GitHub Discussions**: Ask compatibility questions +- **Native Plugins**: See `alternatives.md` in this skillskills/webf-native-plugins/SKILL.md (3)
1-4: Clarify that WebF is a Flutter framework in the skill description.The description uses "standard web APIs" without clarifying that WebF is a Flutter framework that hosts web content. This may confuse users about whether WebF is a traditional web framework.
Consider revising to: "Install WebF native plugins to access platform capabilities (sharing, payment, camera, geolocation, etc.) in your Flutter app. Use when you need native device APIs beyond standard web APIs in your embedded web content."
45-45: Wrap bare URLs in markdown link syntax.Lines 45 and 58 contain bare URLs that should be wrapped in markdown links per MD034 rule.
🔎 Proposed fix
- Visit the official plugin registry: **https://openwebf.com/en/native-plugins** + Visit the official plugin registry: **[https://openwebf.com/en/native-plugins](https://openwebf.com/en/native-plugins)** - Visit https://openwebf.com/en/native-plugins and search for the capability you need: + Visit [https://openwebf.com/en/native-plugins](https://openwebf.com/en/native-plugins) and search for the capability you need:Also applies to: 58-58
219-220: Wrap bare URLs in markdown link syntax.Lines 219 and 220 contain bare URLs that violate MD034 rule.
🔎 Proposed fix
- 1. **Read the Plugin Development Guide**: https://openwebf.com/en/docs/developer-guide/native-plugins - 2. **Study existing plugins**: https://github.com/openwebf/webf/tree/main/webf_modules + 1. **Read the Plugin Development Guide**: [https://openwebf.com/en/docs/developer-guide/native-plugins](https://openwebf.com/en/docs/developer-guide/native-plugins) + 2. **Study existing plugins**: [https://github.com/openwebf/webf/tree/main/webf_modules](https://github.com/openwebf/webf/tree/main/webf_modules)skills/webf-native-plugins/reference.md (2)
223-224: Wrap bare URLs in markdown link syntax.These bare URLs violate the MD034 (no-bare-urls) rule and should be wrapped in markdown link syntax.
🔎 Proposed fix
- 1. **Plugin Development Guide**: https://openwebf.com/en/docs/developer-guide/native-plugins - 2. **Example Plugins**: https://github.com/openwebf/webf/tree/main/webf_modules + 1. **Plugin Development Guide**: [https://openwebf.com/en/docs/developer-guide/native-plugins](https://openwebf.com/en/docs/developer-guide/native-plugins) + 2. **Example Plugins**: [https://github.com/openwebf/webf/tree/main/webf_modules](https://github.com/openwebf/webf/tree/main/webf_modules) - **Plugin Issues**: Report on GitHub at https://github.com/openwebf/webf/issues - **Plugin Requests**: Open a feature request on GitHub - **Documentation**: Visit https://openwebf.com/en/docs + **Plugin Issues**: Report on GitHub at [https://github.com/openwebf/webf/issues](https://github.com/openwebf/webf/issues) + **Plugin Requests**: Open a feature request on GitHub + **Documentation**: Visit [https://openwebf.com/en/docs](https://openwebf.com/en/docs)Also applies to: 274-274, 276-276
295-297: Wrap bare URL in markdown link syntax.Line 297 contains a bare URL that violates MD034 rule.
🔎 Proposed fix
- **Plugin Registry**: https://openwebf.com/en/native-plugins + **Plugin Registry**: [https://openwebf.com/en/native-plugins](https://openwebf.com/en/native-plugins)skills/webf-routing-setup/cross-platform.md (3)
40-50: Clarify that WebF runs Flutter apps, not web apps, in environment detection examples.Lines 40-50 document environment detection but don't clarify that WebF is specifically a Flutter runtime. The note at line 54 hints this ("WebF doesn't need a provider wrapper") but could be more explicit. Readers should understand that WebF renders Flutter screens, not HTML.
🔎 Suggested clarification
Consider adding a comment before the environment detection logic to clarify:
// Detect if running in WebF +// WebF is a W3C-compliant web runtime for Flutter apps +// Browser environments use the standard DOM/History API const isWebF = typeof window !== 'undefined' && typeof (window as any).webf !== 'undefined';And enhance the RouterProvider comment:
// Router provider wrapper export function RouterProvider({ children }) { if (isWebF) { - // WebF doesn't need a provider wrapper + // WebF (Flutter) uses native navigation stack—no provider wrapper needed return <>{children}</>; } - // Browser needs BrowserRouter wrapper + // Browser SPA requires BrowserRouter wrapper for History API integration return <BrowserRouter.BrowserRouter>{children}</BrowserRouter.BrowserRouter>; }
42-42: Address type safety for WebF global detection.The environment detection at line 42 uses
(window as any)to avoid TypeScript errors. While pragmatic, this weakens type safety.🔎 Suggested improvement
Define a module augmentation for better type safety:
// src/types/webf.d.ts declare global { interface Window { webf?: any; } }Then update the detection:
-const isWebF = typeof window !== 'undefined' && typeof (window as any).webf !== 'undefined'; +const isWebF = typeof window !== 'undefined' && typeof window.webf !== 'undefined';This maintains type safety while eliminating the
as anycast. The declaration file is mentioned at line 405 but the augmentation could be shown here.Also applies to: 287-287
101-112: Link component may not prevent default navigation correctly in WebF.The Link component's
handleClickprevents default and callsnavigate.push(), but in WebF, this pattern differs from native navigation. The comment should clarify this is a convenience wrapper, not a native link component.🔎 Suggested clarification
-// Link component that works in both environments +// Link component that works in both environments +// Note: In WebF, this is a JavaScript-based navigation wrapper. +// For true native links, use WebFRouterLink from @openwebf/react-router. export function Link({ to, state, children, ...props }) { const handleClick = (e) => { e.preventDefault(); navigate.push(to, state); }; return ( <a href={to} onClick={handleClick} {...props}> {children} </a> ); }This clarification helps readers understand when to use this wrapper vs. native WebFRouterLink (documented in SKILL.md).
skills/webf-routing-setup/SKILL.md (3)
1-10: Emphasize that WebF is specifically a Flutter runtime.Line 8 mentions WebF differences but doesn't explicitly state that WebF targets Flutter apps. Per learnings, documentation should clarify this distinction. The current text focuses on "native screen transitions" but readers may not understand WebF renders Flutter, not web.
🔎 Suggested clarification
> **Note**: WebF development is nearly identical to web development - you use the same tools (Vite, npm, Vitest), same frameworks (React, Vue, Svelte), and same deployment services (Vercel, Netlify). This skill covers **one of the 3 key differences**: routing with native screen transitions instead of SPA routing. The other two differences are async rendering and API compatibility. +WebF is a **W3C-compliant web runtime for Flutter**. It compiles your web code to Flutter apps for iOS, Android, macOS, Windows, and Linux.This adds crucial context that WebF is a Flutter app builder, not a traditional web framework.
246-273: Advanced navigation methods lack code examples.Lines 246-273 document
popAndPushNamed,maybePop,restorablePopAndPushNamed, and other Flutter-style navigation methods, but provide only API signatures without examples. Readers familiar with WebF may need context on when to use these.🔎 Suggested enhancement
Add brief example scenarios:
+// Common use case: Navigate from product detail back to list with success message +// Use popAndPush to remove detail and add success screen in one action +await WebFRouter.popAndPushNamed('/success', { orderId: 'ORD-123' }); + +// Common use case: Confirm user can navigate back (e.g., from modal) if (WebFRouter.canPop()) { const didPop = WebFRouter.maybePop({ cancelled: false }); console.log('Did pop:', didPop); }This helps readers understand practical scenarios for these advanced methods.
305-351: Protected route pattern may have race condition.The ProtectedRoute at lines 305-351 redirects to login in a useEffect, but the redirect happens after the component renders on first mount. This could cause a brief flash of null content or race conditions if the user state changes during redirect.
🔎 Suggested improvement
Consider whether a synchronous check before rendering is safer:
function ProtectedRoute({ children, isAuthenticated }) { const location = useLocation(); + // Synchronous check before render + if (!isAuthenticated) { + // Redirect immediately without rendering children + // (useEffect will confirm and finalize redirect) + return null; + } + useEffect(() => { if (!isAuthenticated) { WebFRouter.pushState({ redirectTo: location.pathname }, '/login'); } }, [isAuthenticated, location.pathname]); if (!isAuthenticated) { return null; } return children; }Or add a comment noting that a loading spinner is recommended:
if (!isAuthenticated) { return null; // Or loading spinner to prevent flash }skills/webf-routing-setup/examples.md (5)
1-10: Provide guidance on runnable examples and testing.The examples.md file provides five complete examples (lines 1-872) but lacks testing code and setup guidance. Per learnings, examples should be complete and runnable, with tests provided.
🔎 Suggested addition at end of examples
Add a testing section with Vitest examples:
## Testing These Examples ### Test Example 1: Navigation \`\`\`javascript // src/__tests__/navigation.test.js import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import App from '../App'; describe('Navigation', () => { it('navigates from home to profile', async () => { const user = userEvent.setup(); render(<App />); const profileBtn = screen.getByText('View Profile'); await user.click(profileBtn); await waitFor(() => { expect(screen.getByText('Profile Page')).toBeInTheDocument(); }); }); }); \`\`\` This helps readers verify the examples work correctly.Have these examples been tested with actual WebF and browser environments? The learnings indicate "Test all code examples provided in documentation."
137-141: Mock data should be clearly marked and externalized for production patterns.Example 2 at line 137-141 and Example 3 at line 396-418 use hardcoded mock data in component files. While acceptable for documentation, these should note that production code should externalize data loading.
🔎 Suggested improvement
Add comments clarifying these are mocks:
// src/pages/ProductListPage.jsx -const PRODUCTS = [ +// Mock data—in production, fetch from API +const PRODUCTS = [ { id: 1, name: 'Laptop', price: 999.99, description: 'Powerful laptop for work' }, ... ];Or better, show the recommended pattern:
### Production Pattern Move data to a separate module: \`\`\`javascript // src/data/products.js export const PRODUCTS = [...] \`\`\` Then import: \`\`\`jsx import { PRODUCTS } from '../data/products'; \`\`\`This teaches readers scalable patterns.
Also applies to: 396-418
753-808: TabsLayout flex layout may not work optimally on all mobile devices.The TabsLayout component at lines 753-808 uses
display: flexwithflexDirection: 'column'and tab buttons at the bottom. This pattern may cause issues on small screens or with soft keyboards on mobile devices.🔎 Suggested consideration
Add a note about mobile layouts:
function TabsLayout({ children }) { const location = useLocation(); const currentPath = location.pathname; + + // Note: This layout assumes fixed tab bar at bottom. + // On very small screens, consider hiding tabs or using a hamburger menu.And consider a media query example:
// For production, hide tabs on very small screens const tabsStyle = { display: window.innerWidth < 320 ? 'none' : 'flex', // ... };This shows readers how to make the pattern responsive.
587-607: LoginPage mock authentication should clarify expected API integration.Lines 587-607 use simulated login with hardcoded credentials. The comment notes "In real app, call authentication API" but doesn't show the expected pattern or security considerations.
🔎 Suggested enhancement
Add a note about real implementation:
// Simulate login if (email && password) { - // In real app, call authentication API + // In production: + // 1. Call authentication API (with error handling, timeouts) + // 2. Store session token securely (not in state) + // 3. Set up refresh token rotation + // 4. Handle authentication errors gracefully const user = { id: 1, email: email, name: 'John Doe' };Or provide a separate subsection with a real authentication pattern example.
915-946: Testing section provides guidance but no executable test code.Lines 915-946 list what to test (transitions, back button, state passing, deep linking, lifecycle) but don't provide Vitest or React Testing Library examples. Per learnings, provide complete, testable examples.
Would you like me to generate complete Vitest test code for these examples? I can provide:
- Navigation test suite (pushState, back, replaceState)
- State passing test (location.state verification)
- Dynamic routes test (useParams verification)
- Authentication flow test (ProtectedRoute behavior)
- Tabs layout test (active tab styling)
This would align with the learning that all documentation examples should include tests.
skills/webf-quickstart/SKILL.md (5)
8-8: Clarify platform messaging to emphasize Flutter, not web.The statement "nearly identical to building regular web apps" is potentially misleading. WebF builds Flutter apps, not web apps. The distinction is important for developers to understand the platform's native capabilities and limitations.
Consider revising to: "WebF development uses familiar web tools (Vite, React, Vue), but builds native Flutter apps that run on iOS, Android, macOS, Windows, and Linux—not in browsers."
Based on learnings, documentation should clarify that WebF builds Flutter apps, not web apps.
31-31: Wrap bare URLs in markdown link format.Bare URLs should be wrapped in brackets with URL in parentheses for consistency with markdown best practices.
-Get it from: **https://openwebf.com/en/go** +Get it from: **[https://openwebf.com/en/go](https://openwebf.com/en/go)**
67-72: Add language identifier to code block.Specify the language for syntax highlighting clarity. Since this is terminal output, use
bashor leave empty if it's plain output.-Your terminal will show URLs like: -``` - VITE v5.0.0 ready in 123 ms +Your terminal will show URLs like: +```bash + VITE v5.0.0 ready in 123 ms
140-175: Convert bold emphasis to proper markdown headings.Lines 140, 145, 149, and 171 use bold emphasis (
**...**) where semantic headings would improve structure and readability.-**1. Build Your Web Bundle** +## 1. Build Your Web BundleApply similar changes to:
- Line 145: "2. Host Your Bundle"
- Line 149: "3. Create a Flutter App with WebF Integration"
- Line 171: "4. Build and Deploy Flutter App"
218-222: Wrap bare URLs in markdown link format.The Resources section contains bare URLs that should be wrapped in markdown link syntax for consistency.
-- **Getting Started Guide**: https://openwebf.com/en/docs/developer-guide/getting-started -- **WebF Go Guide**: https://openwebf.com/en/docs/learn-webf/webf-go -- **Development Workflow**: https://openwebf.com/en/docs/developer-guide/development-workflow -- **Download WebF Go**: https://openwebf.com/en/go -- **Full Documentation**: https://openwebf.com/en/docs +- **[Getting Started Guide](https://openwebf.com/en/docs/developer-guide/getting-started)** +- **[WebF Go Guide](https://openwebf.com/en/docs/learn-webf/webf-go)** +- **[Development Workflow](https://openwebf.com/en/docs/developer-guide/development-workflow)** +- **[Download WebF Go](https://openwebf.com/en/go)** +- **[Full Documentation](https://openwebf.com/en/docs)**skills/webf-infinite-scrolling/SKILL.md (2)
779-780: Remove absolute file paths; use web URLs or relative paths.The Resources section contains absolute file system paths that are specific to a developer's local machine. These are not portable and should be replaced with relative paths or web URLs.
-- **React Core UI Package**: `/Users/andycall/workspace/webf/packages/react-core-ui/README.md` -- **Vue Core UI Package**: `/Users/andycall/workspace/webf/packages/vue-core-ui/README.md` +- **React Core UI Package**: Visit the project README in `packages/react-core-ui/` +- **Vue Core UI Package**: Visit the project README in `packages/vue-core-ui/`Or link to the published npm packages if available.
783-784: Wrap bare URLs in markdown link format.The npm package URLs should be wrapped in markdown link syntax for consistency.
-- https://www.npmjs.com/package/@openwebf/react-core-ui -- https://www.npmjs.com/package/@openwebf/vue-core-ui +- [@openwebf/react-core-ui on npm](https://www.npmjs.com/package/@openwebf/react-core-ui) +- [@openwebf/vue-core-ui on npm](https://www.npmjs.com/package/@openwebf/vue-core-ui)skills/PUBLISHING.md (1)
101-119: Consider adding language specification to the fenced code block.The CI/CD workflow example is a YAML file but lacks a language identifier. Adding
yamlafter the opening fence improves syntax highlighting and documentation tooling support.🔎 Proposed fix
-``` +```yaml # .github/workflows/publish.ymlcli/test/agents-init.test.ts (1)
69-78: Consider adding error handling test cases.While the happy path is well-tested, consider adding tests for error scenarios such as:
- Missing or uninstalled
@openwebf/claude-code-skillspackage- Permission errors when writing files
- Invalid or corrupted skill files
Example error test case
it('handles missing package gracefully', async () => { // Mock require.resolve to throw jest.mock('@openwebf/claude-code-skills/package.json', () => { throw new Error('Cannot find module'); }); await expect(agentsInitCommand(tempDir)).rejects.toThrow(); });skills/webf-hybrid-ui-dev/SKILL.md (1)
36-48: Consider adding language specification for better rendering.The architecture diagram uses a plain code fence. Adding a language identifier (e.g.,
textorascii) can improve rendering in some documentation viewers.🔎 Proposed fix
-``` +```text ┌─────────────────────────────────────────┐ │ JavaScript/TypeScript (React/Vue) │ ← Generated by CLIskills/README.md (1)
408-420: Add language specifier to fenced code block.The directory structure code block should have a language specifier for consistency and better rendering in some markdown processors.
Suggested fix
-``` +```text my-webf-app/ ├── src/ │ ├── main.jsx # Entry pointskills/webf-hybrid-ui-dev/example-input.md (1)
7-14: Add language specifier to fenced code block.The directory structure code block should have a language specifier for consistency.
Suggested fix
-``` +```text my_component_lib/ └── lib/ └── src/cli/src/agents.ts (4)
201-265: Use logger utility instead of console methods.Per coding guidelines, the CLI module should use the logger utility (
debug,info,warn,errorfrom'./logger') for output instead ofconsole.log. This ensures consistent logging behavior across the CLI.Suggested approach
import fs from 'fs'; import os from 'os'; import path from 'path'; import yaml from 'yaml'; +import { info, debug } from './logger';Then replace
console.logcalls with appropriate logger methods:
- Use
info()for user-facing status messages- Use
debug()for detailed diagnostic outputBased on coding guidelines, CLI TypeScript files should use the logger utility for debugging instead of console methods.
201-265: Add try-catch error handling for robustness.The main
agentsInitCommandfunction performs multiple I/O operations that could fail (file reads, writes, directory operations, package resolution). Per coding guidelines, implement proper error handling with try-catch and add descriptive error messages to help users diagnose issues.Suggested structure
async function agentsInitCommand(projectDir: string): Promise<void> { const startedAt = Date.now(); const resolvedProjectDir = path.resolve(process.cwd(), projectDir || '.'); try { // ... existing implementation ... } catch (error: unknown) { const message = error instanceof Error ? error.message : String(error); throw new Error(`Failed to initialize WebF agents: ${message}`); } }Based on coding guidelines, CLI TypeScript files must implement proper error handling with try-catch and descriptive error messages.
106-112: Avoid silently swallowing YAML parse errors.The empty catch block silently ignores YAML parsing errors. Consider logging a debug message when frontmatter parsing fails to aid troubleshooting.
Suggested fix
try { const parsed = yaml.parse(fm) ?? {}; return { name: parsed?.name, description: parsed?.description }; - } catch { + } catch (e) { + // Invalid YAML frontmatter - return empty metadata return {}; }If using the logger utility as recommended:
- } catch { + } catch (e) { + debug(`Failed to parse skill frontmatter: ${e}`); return {}; }
23-98: Consider using async fs operations for consistency.The helper functions use synchronous fs operations (
fs.mkdirSync,fs.readFileSync,fs.writeFileSync,fs.readdirSync). While this works, the coding guidelines prefer async/await for asynchronous operations. Since the main function is already async, consider usingfs/promisesfor consistency.That said, for a CLI tool that processes files sequentially, synchronous operations are often acceptable for simplicity. This is a low-priority consideration.
| "dependencies": { | ||
| "@microsoft/tsdoc": "^0.15.1", | ||
| "@microsoft/tsdoc-config": "^0.17.1", | ||
| "@openwebf/claude-code-skills": "^1.0.1", |
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:
npm view @openwebf/claude-code-skills versions --jsonRepository: openwebf/webf
Length of output: 170
🌐 Web query:
@openwebf/claude-code-skills npm package versions
💡 Result:
I couldn’t find any published npm package named "@openwebf/claude-code-skills" (no registry entry or GitHub repo matching that scoped name) in searches of the npm registry and Claude/skills indexes. [1][2]
If you want to double-check locally, run:
- npm view @openwebf/claude-code-skills versions
If that command returns a 404 or "package not found", the package hasn't been published under that name. References: npm package searches and Claude skills directories. [1][2]
Sources:
[1] npm registry search and related Claude packages (npmjs.com).
[2] Claude Code / skills directories and hubs (claudecodeplugins.io, claudecode plugins listings).
The dependency @openwebf/claude-code-skills@1.0.1 does not exist on npm and will cause installation to fail.
This package cannot be found in the npm registry. Either publish the package before merging this PR, or remove it from dependencies.
🤖 Prompt for AI Agents
In @cli/package.json around line 42, The dependency entry
"@openwebf/claude-code-skills": "^1.0.1" in package.json is invalid (not on
npm); remove that dependency line or replace it with a valid published
package/version (or move it to a private/internal registry configuration) so
installs won’t fail; update any code importing the symbol (e.g., imports
referencing @openwebf/claude-code-skills) to use the new package name/version or
guard/remove those imports accordingly.
skills/PUBLISHING.md
Outdated
| ### 1. Navigate to skills directory | ||
|
|
||
| ```bash | ||
| cd /Users/andycall/workspace/webf/skills |
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.
Remove hardcoded absolute path.
The example command contains a hardcoded absolute path specific to a local machine. This should use a generic relative path or placeholder.
🔎 Proposed fix
-cd /Users/andycall/workspace/webf/skills
+cd skillsOr use a placeholder:
-cd /Users/andycall/workspace/webf/skills
+cd /path/to/your/webf/skills📝 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.
| cd /Users/andycall/workspace/webf/skills | |
| cd skills |
🤖 Prompt for AI Agents
In @skills/PUBLISHING.md around line 26, The example uses a hardcoded absolute
path "cd /Users/andycall/workspace/webf/skills"; replace it with a generic
relative path or placeholder (e.g., "cd ./skills" or "cd <path-to-repo>/skills")
so the command works on other machines and clearly indicates it should be
adapted to the reader's environment.
| ### Skill Files Location | ||
| All skills are in `/Users/andycall/workspace/webf/skills/`: | ||
| - `webf-quickstart/` - Getting started guide | ||
| - `webf-async-rendering/` - Async rendering patterns | ||
| - `webf-api-compatibility/` - API/CSS compatibility tables | ||
| - `webf-routing-setup/` - Routing setup and examples | ||
| - `webf-infinite-scrolling/` - High-performance scrolling lists | ||
| - `webf-native-ui/` - Native UI components (Cupertino UI for iOS) | ||
| - `webf-native-plugins/` - Native platform plugins (Share, Camera, etc.) | ||
| - `webf-hybrid-ui-dev/` - Developing custom hybrid UI libraries | ||
|
|
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.
Remove hardcoded local path.
The skill files location section contains a hardcoded absolute path (/Users/andycall/workspace/webf/skills/) that is specific to a developer's machine. This should be a relative path or a generic placeholder.
Suggested fix
### Skill Files Location
-All skills are in `/Users/andycall/workspace/webf/skills/`:
+All skills are in `.claude/skills/` (installed via `webf agents init`):
- `webf-quickstart/` - Getting started guide
- `webf-async-rendering/` - Async rendering patterns📝 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.
| ### Skill Files Location | |
| All skills are in `/Users/andycall/workspace/webf/skills/`: | |
| - `webf-quickstart/` - Getting started guide | |
| - `webf-async-rendering/` - Async rendering patterns | |
| - `webf-api-compatibility/` - API/CSS compatibility tables | |
| - `webf-routing-setup/` - Routing setup and examples | |
| - `webf-infinite-scrolling/` - High-performance scrolling lists | |
| - `webf-native-ui/` - Native UI components (Cupertino UI for iOS) | |
| - `webf-native-plugins/` - Native platform plugins (Share, Camera, etc.) | |
| - `webf-hybrid-ui-dev/` - Developing custom hybrid UI libraries | |
| ### Skill Files Location | |
| All skills are in `.claude/skills/` (installed via `webf agents init`): | |
| - `webf-quickstart/` - Getting started guide | |
| - `webf-async-rendering/` - Async rendering patterns | |
| - `webf-api-compatibility/` - API/CSS compatibility tables | |
| - `webf-routing-setup/` - Routing setup and examples | |
| - `webf-infinite-scrolling/` - High-performance scrolling lists | |
| - `webf-native-ui/` - Native UI components (Cupertino UI for iOS) | |
| - `webf-native-plugins/` - Native platform plugins (Share, Camera, etc.) | |
| - `webf-hybrid-ui-dev/` - Developing custom hybrid UI libraries |
🤖 Prompt for AI Agents
In @skills/README.md around lines 547-557, The README currently hardcodes an
absolute path "/Users/andycall/workspace/webf/skills/"; replace it with a
relative or generic placeholder (e.g., "./skills/" or "<repo-root>/skills/") in
the "Skill Files Location" section and update the surrounding text so the list
entries (webf-quickstart/, webf-async-rendering/, etc.) remain the same; ensure
no other absolute local paths remain in that section and keep the phrasing
generic for any contributor.
| ## Resources | ||
|
|
||
| - **Core Concepts - Async Rendering**: https://openwebf.com/en/docs/developer-guide/core-concepts#async-rendering | ||
| - **Debugging & Performance**: https://openwebf.com/en/docs/developer-guide/debugging-performance | ||
| - **@openwebf/react-core-ui**: Install with `npm install @openwebf/react-core-ui` | ||
|
|
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.
Fix bare URLs in Resources section.
Wrap bare URLs in markdown link syntax to comply with markdown standards:
- **Core Concepts - Async Rendering**: https://openwebf.com/en/docs/developer-guide/core-concepts#async-rendering
- **Debugging & Performance**: https://openwebf.com/en/docs/developer-guide/debugging-performance
+ **Core Concepts - Async Rendering**: [https://openwebf.com/en/docs/developer-guide/core-concepts#async-rendering](https://openwebf.com/en/docs/developer-guide/core-concepts#async-rendering)
+ **Debugging & Performance**: [https://openwebf.com/en/docs/developer-guide/debugging-performance](https://openwebf.com/en/docs/developer-guide/debugging-performance)🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
302-302: Bare URL used
(MD034, no-bare-urls)
303-303: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In @skills/webf-async-rendering/SKILL.md around lines 300-305, Replace the bare
URLs under the "Resources" section with proper Markdown link syntax: turn each
bullet that currently shows a plain URL into a bracketed link label followed by
the URL in parentheses (e.g., for "Core Concepts - Async Rendering", "Debugging
& Performance", and the "@openwebf/react-core-ui" install note) so all three
entries are formatted as Markdown links rather than raw URLs.
| ## HTTPS for Mobile Testing | ||
|
|
||
| Some APIs require HTTPS. To enable: | ||
|
|
||
| ```bash | ||
| npm install -D @vitejs/plugin-basic-ssl | ||
| ``` | ||
|
|
||
| ```js | ||
| // vite.config.js | ||
| import basicSsl from '@vitejs/plugin-basic-ssl' | ||
|
|
||
| export default { | ||
| plugins: [basicSsl()], | ||
| server: { | ||
| https: true | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Then use `https://192.168.x.x:5173` instead of `http://` |
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:
# First, verify the file exists and check the content at the specified lines
if [ -f "skills/webf-quickstart/reference.md" ]; then
sed -n '89,109p' skills/webf-quickstart/reference.md
else
echo "File not found, searching for reference.md files..."
find . -name "reference.md" -type f 2>/dev/null | head -20
fiRepository: openwebf/webf
Length of output: 170
🌐 Web query:
Vite HTTPS development setup @vitejs/plugin-basic-ssl official recommendation
💡 Result:
Short answer — official stance:
- @vitejs/plugin-basic-ssl simply auto-generates a self-signed (untrusted) cert for basic local HTTPS. Use is OK for quick local testing but the cert is untrusted. [1].
- Vite’s docs explicitly recommend creating and using your own trusted certificates (or supplying a valid cert via server.https) for HTTPS instead of relying on the plugin; the plugin is provided only to restore the old automatic behavior. Example: add basicSsl() to plugins to re-enable auto-generation, or pass server.https an https.ServerOptions with your cert/key. [2].
References:
- @vitejs/plugin-basic-ssl README (plugin behavior and recommendation). [1]
- Vite server.https docs (recommendation to create/use your own certificates; how to enable plugin for basic setup). [2]
🌐 Web query:
@vitejs/plugin-basic-ssl npm package maintenance status 2025
💡 Result:
Short answer: Actively maintained in 2025 — the package had multiple releases in 2025 (v2.0.0 and v2.1.0), the npm package was published/upgraded in 2025, and the GitHub repo shows ongoing activity and open issues/PRs. [1][2][3]
Sources:
- npm package page (current version v2.1.0, recent publish). [1]
- GitHub repo for vite-plugin-basic-ssl (active repo / issues). [2]
- Release/changelog entries showing v2.0.0 (Mar 3, 2025) and v2.1.0 (Jul 1, 2025). [3]
The @vitejs/plugin-basic-ssl package is current, but Vite's official documentation recommends using your own trusted certificates instead.
The package is actively maintained (v2.1.0, July 2025) and works for local development. However, Vite's documentation recommends creating or supplying your own certificates via server.https rather than relying on the plugin's auto-generated self-signed certificates. The plugin creates untrusted certificates suitable only for local testing, which may trigger browser warnings. Consider clarifying in the guide whether the plugin approach is acceptable for the WebF mobile testing workflow, or recommend users generate trusted certificates if strict HTTPS validation is needed.
🤖 Prompt for AI Agents
In @skills/webf-quickstart/reference.md around lines 89-109, The docs currently
recommend installing @vitejs/plugin-basic-ssl and enabling server.https with
basicSsl(), but you must clarify that the plugin generates self-signed,
untrusted certificates (browser warnings) and that Vite recommends supplying
your own trusted certs via the server.https option; update the vite.config.js
section to note the plugin is OK for quick local testing but advise users who
need trusted HTTPS to generate/provide certificate and key files and configure
server.https accordingly (or link to Vite’s docs for server.https and steps to
create trusted certs), and explicitly state whether the plugin approach is
acceptable for WebF mobile testing vs when to use real certs.
50fe409 to
31f18f6
Compare
31f18f6 to
20a381a
Compare
Summary by CodeRabbit
New Features
Bug Fixes
Removed Features
✏️ Tip: You can customize this high-level summary in your review settings.