Repository navigation
chore: update to latest Qwik v2 - #136
Conversation
dosubot
Bot
added
the
size:L
|
@maiieul is attempting to deploy a commit to the Open Circle Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR upgrades 🚥 Pre-merge checks | ✅ 5
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.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the Qwik integration and examples to align with newer Qwik signal typing/behavior and updated tooling dependencies.
Changes:
- Replaced
ReadonlySignal<T>usages withReadonly<Signal<T>>across docs, playground components, and framework Qwik types. - Updated Qwik/Vite-related devDependencies (including switching some Qwik deps to
pkg.pr.newURLs) and removed a pnpm patched dependency entry. - Adjusted Qwik router preview entry and tweaked
useResolvedQrlto avoid tracked reads.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| website/src/routes/(docs)/qwik/guides/(main-concepts)/input-components/index.mdx | Updates guide sample types/imports to new Signal typing pattern. |
| website/package.json | Updates Qwik deps to pkg.pr.new URLs, bumps eslint-plugin-qwik and Vite. |
| playgrounds/qwik/src/entry.preview.tsx | Removes router config import/parameter from preview adapter. |
| playgrounds/qwik/src/components/TextInput.tsx | Migrates component prop types from ReadonlySignal to Readonly<Signal>. |
| playgrounds/qwik/src/components/Slider.tsx | Migrates component prop types from ReadonlySignal to Readonly<Signal>. |
| playgrounds/qwik/src/components/Select.tsx | Migrates component prop types from ReadonlySignal to Readonly<Signal>. |
| playgrounds/qwik/src/components/RadioGroup.tsx | Migrates component prop types from ReadonlySignal to Readonly<Signal>. |
| playgrounds/qwik/src/components/InputErrors.tsx | Migrates component prop types from ReadonlySignal to Readonly<Signal>. |
| playgrounds/qwik/src/components/FileInput.tsx | Migrates component prop types from ReadonlySignal to Readonly<Signal>. |
| playgrounds/qwik/src/components/Checkbox.tsx | Migrates component prop types from ReadonlySignal to Readonly<Signal>. |
| playgrounds/qwik/package.json | Updates Qwik deps to pkg.pr.new URLs, bumps eslint-plugin-qwik and Vite. |
| packages/core/package.json | Updates @qwik.dev/core to pkg.pr.new URL. |
| package.json | Removes pnpm patchedDependencies block for @qwik.dev/router. |
| frameworks/qwik/src/types/form.ts | Migrates exported form store signal fields to Readonly<Signal<...>>. |
| frameworks/qwik/src/types/field.ts | Migrates exported field store signal fields to Readonly<Signal<...>>. |
| frameworks/qwik/src/hooks/useResolvedQrl/useResolvedQrl.ts | Uses untrack to avoid tracked reads when resolving QRLs. |
| frameworks/qwik/src/hooks/usePathSignal/usePathSignal.ts | Returns Readonly<Signal<T>> instead of ReadonlySignal<T>. |
| frameworks/qwik/package.json | Updates @qwik.dev/core to pkg.pr.new URL, bumps eslint-plugin-qwik and Vite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ed7441a to
ff6f3d9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
playgrounds/qwik/src/components/InputErrors.tsx (1)
5-8: ⚡ Quick winUse
interfaceforInputErrorPropsobject shape.This props object is declared with
type, but the TypeScript guideline here requiresinterfacefor object shapes.Suggested change
-type InputErrorProps = { +interface InputErrorProps { name: string; errors: Readonly<Signal<[string, ...string[]] | null>>; -}; +}As per coding guidelines, "Prefer
interfaceovertypefor defining object shapes in TypeScript".🤖 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 `@playgrounds/qwik/src/components/InputErrors.tsx` around lines 5 - 8, Change the props declaration from a type alias to an interface: replace the type InputErrorProps definition with an interface InputErrorProps that declares the same members (name: string; errors: Readonly<Signal<[string, ...string[]] | null>>;), ensuring any usages of InputErrorProps (e.g., component props or function signatures) remain unchanged.Source: Coding guidelines
🤖 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 `@frameworks/qwik/package.json`:
- Line 50: Multiple workspace packages are pinned to different Qwik commits;
unify them by choosing one canonical commit SHA (e.g., the one used in
website/package.json or the desired release) and update all occurrences of
`@qwik.dev/core` and `@qwik.dev/router` in frameworks/qwik/package.json,
website/package.json (and any other package manifests) to reference that same
commit tag, then regenerate and commit an updated pnpm-lock.yaml by running pnpm
install so the lockfile matches the manifests; ensure you update the dependency
strings that reference the pkg.pr.new URLs (the `@qwik.dev/core` and
`@qwik.dev/router` entries) and verify the workspace builds.
In `@playgrounds/qwik/src/entry.preview.tsx`:
- Around line 14-15: The top-of-file comment "make sure qwikRouterConfig is
imported before entry" is stale; remove or update it to reflect current imports
in entry.preview.tsx (there is only the render import from './entry.ssr'). Edit
the file to delete the obsolete comment or replace it with a current, accurate
note about import ordering if needed, ensuring references to qwikRouterConfig
are removed and only relevant context for import render is kept.
- Line 20: The pure factory call createQwikRouter({ render }) needs the
tree-shaking purity annotation; add the comment // `@__NO_SIDE_EFFECTS__`
immediately on the line above the createQwikRouter invocation (the default
export) so the pure factory call createQwikRouter is annotated for tree-shaking
optimization.
In
`@website/src/routes/`(docs)/qwik/guides/(main-concepts)/input-components/index.mdx:
- Around line 69-77: The props interface TextInputProps now types input and
errors as Readonly<Signal<...>> but the usage snippets still pass
field.input.value / field.errors.value; update the example(s) that consume
TextInputProps (references: TextInputProps, input, errors, and
FieldElementProps) to pass the signal objects themselves (e.g., field.input,
field.errors) instead of their .value, so the example compiles; apply the same
change to the other occurrence noted around lines 145-153.
---
Nitpick comments:
In `@playgrounds/qwik/src/components/InputErrors.tsx`:
- Around line 5-8: Change the props declaration from a type alias to an
interface: replace the type InputErrorProps definition with an interface
InputErrorProps that declares the same members (name: string; errors:
Readonly<Signal<[string, ...string[]] | null>>;), ensuring any usages of
InputErrorProps (e.g., component props or function signatures) remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e4b5f13-eca2-473d-bc2e-b85511088c81
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
frameworks/qwik/package.jsonframeworks/qwik/src/hooks/usePathSignal/usePathSignal.tsframeworks/qwik/src/hooks/useResolvedQrl/useResolvedQrl.tsframeworks/qwik/src/types/field.tsframeworks/qwik/src/types/form.tspackage.jsonpackages/core/package.jsonplaygrounds/qwik/package.jsonplaygrounds/qwik/src/components/Checkbox.tsxplaygrounds/qwik/src/components/FileInput.tsxplaygrounds/qwik/src/components/InputErrors.tsxplaygrounds/qwik/src/components/RadioGroup.tsxplaygrounds/qwik/src/components/Select.tsxplaygrounds/qwik/src/components/Slider.tsxplaygrounds/qwik/src/components/TextInput.tsxplaygrounds/qwik/src/entry.preview.tsxwebsite/package.jsonwebsite/src/routes/(docs)/qwik/guides/(main-concepts)/input-components/index.mdx
💤 Files with no reviewable changes (1)
- package.json
ff6f3d9 to
37c6eee
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found and verified against the latest diff
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
9ebb679 to
9d92871
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@pnpm-workspace.yaml`:
- Line 52: The vite@7 override in pnpm-workspace.yaml is pinned to version
7.3.2, which is downgrading the intended upgrade to 7.3.5 specified in the
package manifests. Update the version value for the vite@7 override entry from
7.3.2 to 7.3.5 to align with the manifest bumps and allow packages like website
and frameworks/qwik to use the newer version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a505646b-58ed-454f-84bd-4ac624265c34
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
frameworks/qwik/package.jsonframeworks/qwik/src/hooks/usePathSignal/usePathSignal.tsframeworks/qwik/src/types/field.tsframeworks/qwik/src/types/form.tspackages/core/package.jsonplaygrounds/qwik/package.jsonplaygrounds/qwik/src/components/Checkbox.tsxplaygrounds/qwik/src/components/FileInput.tsxplaygrounds/qwik/src/components/InputErrors.tsxplaygrounds/qwik/src/components/RadioGroup.tsxplaygrounds/qwik/src/components/Select.tsxplaygrounds/qwik/src/components/Slider.tsxplaygrounds/qwik/src/components/TextInput.tsxplaygrounds/qwik/src/entry.preview.tsxpnpm-workspace.yamlwebsite/package.jsonwebsite/src/routes/(docs)/qwik/guides/(main-concepts)/input-components/index.mdx
✅ Files skipped from review due to trivial changes (1)
- playgrounds/qwik/src/components/Slider.tsx
🚧 Files skipped from review as they are similar to previous changes (13)
- frameworks/qwik/package.json
- playgrounds/qwik/src/components/RadioGroup.tsx
- packages/core/package.json
- frameworks/qwik/src/hooks/usePathSignal/usePathSignal.ts
- playgrounds/qwik/src/components/FileInput.tsx
- website/src/routes/(docs)/qwik/guides/(main-concepts)/input-components/index.mdx
- playgrounds/qwik/src/components/TextInput.tsx
- playgrounds/qwik/src/entry.preview.tsx
- frameworks/qwik/src/types/field.ts
- playgrounds/qwik/src/components/Checkbox.tsx
- frameworks/qwik/src/types/form.ts
- playgrounds/qwik/src/components/Select.tsx
- playgrounds/qwik/src/components/InputErrors.tsx
|
There stills seems to be the bug where pages render the same content multiple times when navigating: 
|
|
It's a Qwik core projection-diffing bug (triggered by our |
|
Ah my bad I thought it was in 😅. Yeah we can wait for beta 38. |
dosubot
Bot
added
size:XL
Status: blocked on two Qwik v2.0.0-beta.38 issuesWhile finalizing this PR on beta.38, we hit two upstream regressions: 1. The rule only recognizes signals by a type name ending in Workaround in this PR: 2. SSG no longer renders On beta.38, the website's prerendered pages are missing all menu-driven UI (docs sidebar, API reference lists). The loader runs fine and its value is present in the serialized container state, but components rendering Everything else is green (core/methods/qwik builds, tests, lints, playground verified in dev and preview — the double-render on navigation is fixed). We'll hold off merging until the SSG issue is resolved upstream. |
a2ce0d4 to
a9175b2
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
a9175b2 to
2fb8e86
Compare
2fb8e86 to
aabf51d
Compare
There was a problem hiding this comment.
All reported issues were addressed across 57 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ages - Bump @qwik.dev/core, @qwik.dev/router and eslint-plugin-qwik to 2.0.0-beta.38 - Add signal.qwik.ts framework override so the core Signal type matches Qwik's new Signal interface (now includes untrackedValue and trigger) - Migrate website components from deprecated ReadonlySignal<T> to Readonly<Signal<T>> (matches the playground migration) - Update website API docs for FormStore, FieldStore and FieldArrayStore - Remove removed qwikRouterConfig from website entry.preview.tsx
renderToStream resolves the client manifest internally via getClientManifest() since Qwik v2.0.0-beta.38.
Add a Formisch-owned ReadonlySignal type to @formisch/qwik and use it in place of Readonly<Signal<T>> across types, hooks, playground, website components and docs. This works around a false positive in eslint-plugin-qwik's valid-lexical-scope rule, which only recognizes signals by a type name ending in "Signal" and therefore flags Readonly<Signal<T>> captures as unserializable. Remove once fixed upstream.
6d2a37e to
4bfbfdd
Compare
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Change the Qwik peer dependency of @formisch/qwik and @formisch/core to >=2.0.0-0 <3, because the previous range >=2 <3 did not match any Qwik v2 beta or release candidate, which makes npm fail with ERESOLVE - Use Vite's native tsconfig paths resolution in the Qwik playground and remove its now unused vite-tsconfig-paths dependency - Remove the unused vite-imagetools dependency from the website
Qwik v2 computed signals recompute synchronously and Formisch's Qwik batch is a no-op, so the form's computed state like isValid walked the new items of a field array before their child stores were created and crashed with "Cannot read properties of undefined". Update the items signal only after the child stores are created in insert, copyItemState, swapItemState and resetItemState.
Qwik Router v2 fetches routeLoader$ data from per-loader JSON files during client navigation. For loaders in the root plugin, this is a single URL that a static host can't serve per page, and with strict loaders the loaders no longer receive the page pathname, so the docs menu was missing. - Replace the useMenu loader with a hook based on useContent().menu, which resolves the nearest menu.md without a loader - Replace the useOtherMenuHrefs loader with a lookup of the globbed menu.md files in the framework picker - Mark the blog loader as immutable so SSG writes its data file - Pass the client manifest hash to SSG, because Qwik Router does not pass it and the loader data files would otherwise not match the URLs requested during client navigation
The framework picker eagerly imported all 16 compiled menu modules on the client and rebuilt their href lists on every render, although it only needs to know whether a page exists for another framework. Add a small Vite plugin that reads the menu.md files at build time and provides only their hrefs as a Set via the virtual:menu-hrefs module. This reduces the client code for this lookup from about 8 KB to 1.8 KB gzipped and replaces the lookup with a single Set check.
4bfbfdd to
8ecc243
Compare
Summary by cubic
Upgrade Qwik to v2.0.0-beta.37 and migrate to the new Signal API. Fix the double-render in
pkg.pr.newpreview by removing the extra router config.Dependencies
@qwik.dev/core,@qwik.dev/router, andeslint-plugin-qwikto2.0.0-beta.37across the repo.viteto7.3.5andvue-tscto~3.1.0.pnpm.patchedDependenciesfor@qwik.dev/router.Migration
ReadonlySignal<T>withReadonly<Signal<T>>across hooks, types, components, and docs.export default createQwikRouter({ render }); removeqwikRouterConfig.Written for commit dacb579. Summary will update on new commits.