fix: migrate partytown @builder.io → @qwik.dev@0.14.0#3344
fix: migrate partytown @builder.io → @qwik.dev@0.14.0#3344renatomaurovtex wants to merge 6 commits into
Conversation
|
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:
WalkthroughMigrates the ChangesPartytown Package Migration
experimental.optimizedFonts Opt-in
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
@faststore/api
@faststore/cli
@faststore/components
@faststore/core
@faststore/diagnostics
@faststore/lighthouse
@faststore/sdk
@faststore/ui
commit: |
…onts (#3351) ## Summary Adds a new opt-in config flag `experimental.optimizedFonts` (default `false`). When enabled, FastStore self-hosts the Inter font via `@fontsource/inter`, eliminating the render-blocking request to `fonts.googleapis.com` and improving FCP by ~660ms on Lighthouse mobile. This is the fourth slice of #3345. > **Stacked PR.** Base branch is `fix/vte-18-partytown-migration` (#3344), so the `pnpm-lock.yaml` diff here only shows the `@fontsource/inter` entries instead of the full lockfile reformat that the partytown PR already includes. After #3344 lands, this PR will be rebased onto `dev` automatically and the diff against `dev` will remain minimal. ## How it works - `discovery.config.default.js` exposes `experimental.optimizedFonts: false`. - `packages/core/src/fonts/inter.ts` is an empty stub. When the flag is `true`, `next.config.js` uses webpack's `NormalModuleReplacementPlugin` to rewrite imports of `src/fonts/inter` to `packages/core/fonts/inter.ts` (located **outside** `src/`, so Babel never scans it). - `packages/core/fonts/inter.ts` side-effect imports the `@fontsource/inter` CSS files (400/500/600/700/900), which ship the self-hosted Inter `.woff2` assets. - `_app.tsx` imports `src/fonts/inter` because Next.js requires global CSS imports to live in `_app.tsx`. - When the flag is `false` (default), webpack never bundles the package — **zero cost when off**. ## Why plain CSS imports instead of `next/font/google` The first attempt used `next/font/google`, which fails at build time for stores shipping a custom `.babelrc.js` (Next.js disables SWC, and `next/font` requires SWC). `@fontsource/inter` is compiler-agnostic plain CSS + woff2, so it works with both Babel and SWC. ## Dependency justification (per [AGENTS.md > Dependency Discipline](https://github.com/vtex/faststore/blob/dev/AGENTS.md#dependency-discipline)) Recorded in detail in `packages/core/CHANGELOG.md`: - **Need:** powers the opt-in `experimental.optimizedFonts` path; required by the Babel-safe constraint above. - **Evaluation:** MIT-licensed, actively maintained (5.x, regular releases), no critical CVEs, ships its own types/exports. - **Bundle impact:** zero when the flag is off — `NormalModuleReplacementPlugin` keeps it out of the module graph. - **Bucket:** `dependencies` (must be resolvable when stores opt in). - **Pinning:** declared locally in `packages/core/package.json`, caret-pinned to `^5.2.6`. Requesting maintainer approval per the Agent Autonomy Boundaries (new runtime dependency in a published package). ## How to enable in a store 1. In `discovery.config.js`: ```js experimental: { optimizedFonts: true, } ``` 2. Remove any `<link rel="stylesheet" href="https://fonts.googleapis.com/...">` from `customizations/src/fonts/WebFonts.tsx` to avoid duplicate font loads. If the store uses a font other than Inter, **do not enable this flag** — keep the existing `WebFonts.tsx` configuration. ## Test plan - [ ] With `optimizedFonts: false` (default): build is byte-identical to before, no `.woff2` in the bundle. - [ ] With `optimizedFonts: true`: `.woff2` files ship, no request to `fonts.googleapis.com`, Inter renders correctly across weights 400/500/600/700/900. - [ ] Babel store (custom `.babelrc.js`) builds cleanly with the flag on. - [ ] SWC store builds cleanly with the flag on. - [ ] `pnpm test --filter=@faststore/core` passes (new tests under `test/fonts/`). - [ ] Lighthouse mobile FCP improves on a real store with the flag enabled. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an opt-in experimental `optimizedFonts` flag to self-host the Inter font (defaults to false), enabling optional local font delivery to reduce external requests. * **Documentation** * Updated changelog with guidance for enabling the font optimization and related setup notes. * **Chores** * Added runtime dependency to support self-hosted Inter fonts. * **Tests** * Added unit tests to verify the default flag value and font-module behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/discovery.config.default.js (1)
206-221: Note: Production config change requiring review.This file matches the pattern
packages/core/discovery.config*.jsspecified in the coding guidelines. The new experimental flag defaults tofalse(opt-in only), which minimizes risk, but careful review is warranted. As per coding guidelines, "Modifying authentication, authorization, CSP, CI/CD pipelines, or production env config requires explicit human approval."The documentation is comprehensive and the default value ensures backward compatibility.
🤖 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 `@packages/core/discovery.config.default.js` around lines 206 - 221, This change introduces a production-facing config flag optimizedFonts (in packages/core/discovery.config*.js) and per guidelines requires explicit human approval; update the PR and the file to signal that: add a clear top-of-file comment and PR checklist entry stating "Requires explicit production config approval" and reference the optimizedFonts flag, and ensure the PR includes an explicit approver (e.g., add a REQUIRED_APPROVAL tag or note in the PR description and notify the ops/security owner) before merging so the production config change is reviewed by the appropriate human.
🤖 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 `@packages/core/next.config.js`:
- Around line 89-108: When optimizedFonts is enabled we currently add a
webpack.NormalModuleReplacementPlugin but don’t add the equivalent Turbopack
alias, so Turbopack still resolves the empty src/fonts/inter stub; add a
Turbopack resolve alias mapping (turbopack.resolveAlias or
experimental.turbopack.resolveAlias depending on existing config structure) that
maps "src/fonts/inter" to the same path.resolve(__dirname, 'fonts/inter.ts')
used in the webpack plugin so Turbopack will bundle the self-hosted Inter
CSS/woff2; keep this addition guarded by the same
storeConfig.experimental?.optimizedFonts === true check.
---
Nitpick comments:
In `@packages/core/discovery.config.default.js`:
- Around line 206-221: This change introduces a production-facing config flag
optimizedFonts (in packages/core/discovery.config*.js) and per guidelines
requires explicit human approval; update the PR and the file to signal that: add
a clear top-of-file comment and PR checklist entry stating "Requires explicit
production config approval" and reference the optimizedFonts flag, and ensure
the PR includes an explicit approver (e.g., add a REQUIRED_APPROVAL tag or note
in the PR description and notify the ops/security owner) before merging so the
production config change is reviewed by the appropriate human.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 911765bd-55ab-471b-a316-56fa34443b3c
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by nonepnpm-workspace.yamlis excluded by none and included by none
📒 Files selected for processing (9)
packages/core/CHANGELOG.mdpackages/core/discovery.config.default.jspackages/core/fonts/inter.tspackages/core/next.config.jspackages/core/package.jsonpackages/core/src/fonts/inter.tspackages/core/src/pages/_app.tsxpackages/core/test/fonts/inter.test.tspackages/core/test/fonts/optimizedFonts.test.ts
✅ Files skipped from review due to trivial changes (3)
- packages/core/fonts/inter.ts
- packages/core/src/fonts/inter.ts
- packages/core/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/package.json
Replace @builder.io/partytown@^0.6.1 with @qwik.dev/partytown@^0.14.0 in packages/core/package.json and update the issue comment URL in lighthouserc.js from BuilderIO/partytown to QwikDev/partytown. Closes VTE-18 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onts (#3351) Adds a new opt-in config flag `experimental.optimizedFonts` (default `false`). When enabled, FastStore self-hosts the Inter font via `@fontsource/inter`, eliminating the render-blocking request to `fonts.googleapis.com` and improving FCP by ~660ms on Lighthouse mobile. This is the fourth slice of #3345. > **Stacked PR.** Base branch is `fix/vte-18-partytown-migration` (#3344), so the `pnpm-lock.yaml` diff here only shows the `@fontsource/inter` entries instead of the full lockfile reformat that the partytown PR already includes. After #3344 lands, this PR will be rebased onto `dev` automatically and the diff against `dev` will remain minimal. - `discovery.config.default.js` exposes `experimental.optimizedFonts: false`. - `packages/core/src/fonts/inter.ts` is an empty stub. When the flag is `true`, `next.config.js` uses webpack's `NormalModuleReplacementPlugin` to rewrite imports of `src/fonts/inter` to `packages/core/fonts/inter.ts` (located **outside** `src/`, so Babel never scans it). - `packages/core/fonts/inter.ts` side-effect imports the `@fontsource/inter` CSS files (400/500/600/700/900), which ship the self-hosted Inter `.woff2` assets. - `_app.tsx` imports `src/fonts/inter` because Next.js requires global CSS imports to live in `_app.tsx`. - When the flag is `false` (default), webpack never bundles the package — **zero cost when off**. The first attempt used `next/font/google`, which fails at build time for stores shipping a custom `.babelrc.js` (Next.js disables SWC, and `next/font` requires SWC). `@fontsource/inter` is compiler-agnostic plain CSS + woff2, so it works with both Babel and SWC. Discipline](https://github.com/vtex/faststore/blob/dev/AGENTS.md#dependency-discipline)) Recorded in detail in `packages/core/CHANGELOG.md`: - **Need:** powers the opt-in `experimental.optimizedFonts` path; required by the Babel-safe constraint above. - **Evaluation:** MIT-licensed, actively maintained (5.x, regular releases), no critical CVEs, ships its own types/exports. - **Bundle impact:** zero when the flag is off — `NormalModuleReplacementPlugin` keeps it out of the module graph. - **Bucket:** `dependencies` (must be resolvable when stores opt in). - **Pinning:** declared locally in `packages/core/package.json`, caret-pinned to `^5.2.6`. Requesting maintainer approval per the Agent Autonomy Boundaries (new runtime dependency in a published package). 1. In `discovery.config.js`: ```js experimental: { optimizedFonts: true, } ``` 2. Remove any `<link rel="stylesheet" href="https://fonts.googleapis.com/...">` from `customizations/src/fonts/WebFonts.tsx` to avoid duplicate font loads. If the store uses a font other than Inter, **do not enable this flag** — keep the existing `WebFonts.tsx` configuration. - [ ] With `optimizedFonts: false` (default): build is byte-identical to before, no `.woff2` in the bundle. - [ ] With `optimizedFonts: true`: `.woff2` files ship, no request to `fonts.googleapis.com`, Inter renders correctly across weights 400/500/600/700/900. - [ ] Babel store (custom `.babelrc.js`) builds cleanly with the flag on. - [ ] SWC store builds cleanly with the flag on. - [ ] `pnpm test --filter=@faststore/core` passes (new tests under `test/fonts/`). - [ ] Lighthouse mobile FCP improves on a real store with the flag enabled. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **New Features** * Added an opt-in experimental `optimizedFonts` flag to self-host the Inter font (defaults to false), enabling optional local font delivery to reduce external requests. * **Documentation** * Updated changelog with guidance for enabling the font optimization and related setup notes. * **Chores** * Added runtime dependency to support self-hosted Inter fonts. * **Tests** * Added unit tests to verify the default flag value and font-module behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…lData instead of webpack rule surgery
665d15c to
06c7027
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 `@packages/core/CHANGELOG.md`:
- Around line 6-88: Resolve the Git merge conflict in the CHANGELOG.md file by
removing the conflict markers (<<<<<<< HEAD, =======, and >>>>>>> 665d15ca7).
Keep the detailed changelog entries from the HEAD branch that document versions
4.3.0-dev.4 through 4.1.2-dev.6, as they provide the complete release history
and should not be deleted. Ensure the final file contains only the changelog
entries without any conflict markers.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 93bcafea-8e0b-4565-b280-468a3128e4a9
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by nonepnpm-workspace.yamlis excluded by none and included by none
📒 Files selected for processing (8)
packages/core/CHANGELOG.mdpackages/core/discovery.config.default.jspackages/core/lighthouserc.jspackages/core/next.config.jspackages/core/package.jsonpackages/core/src/pages/_app.tsxpackages/core/test/fonts/inter.test.tspackages/core/test/fonts/optimizedFonts.test.ts
✅ Files skipped from review due to trivial changes (3)
- packages/core/lighthouserc.js
- packages/core/src/pages/_app.tsx
- packages/core/test/fonts/inter.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/discovery.config.default.js
- packages/core/next.config.js
| @@ -83,6 +84,8 @@ See [Conventional Commits](https://conventionalcommits.org) for commit guideline | |||
|
|
|||
| **Note:** Version bump only for package @faststore/core | |||
|
|
|||
| ======= | |||
| >>>>>>> 665d15ca7 (chore(core): update CHANGELOG) | |||
There was a problem hiding this comment.
Unresolved merge conflict in CHANGELOG.md must be resolved before merge.
Lines 6–88 contain unresolved Git conflict markers (<<<<<<< HEAD, =======, >>>>>>>). The CHANGELOG cannot be published in this state. Resolve the conflict by selecting which entries to keep, merging them chronologically, or adopting the entries from the winning branch.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 19-19: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 25-25: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 35-35: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 46-46: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 52-52: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 62-62: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 69-69: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 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 `@packages/core/CHANGELOG.md` around lines 6 - 88, Resolve the Git merge
conflict in the CHANGELOG.md file by removing the conflict markers (<<<<<<<
HEAD, =======, and >>>>>>> 665d15ca7). Keep the detailed changelog entries from
the HEAD branch that document versions 4.3.0-dev.4 through 4.1.2-dev.6, as they
provide the complete release history and should not be deleted. Ensure the final
file contains only the changelog entries without any conflict markers.
Source: Linters/SAST tools

0 New Issues
0 Fixed Issues
0 Accepted Issues
No data about coverage (19.60% Estimated after merge)
Summary
@builder.io/partytown@^0.6.1with@qwik.dev/partytown@^0.14.0inpackages/core/package.jsonpackages/core/lighthouserc.jsfromBuilderIO/partytowntoQwikDev/partytownpnpm install— lock file updated, old package fully replacedTest plan
@qwik.dev/partytown@0.14.0resolves inpnpm-lock.yaml✅@builder.io/partytownno longer present in lock file ✅pnpm installcompletes without new errors ✅Closes VTE-18
🤖 Generated with Claude Code
Summary by CodeRabbit
experimental.optimizedFontsoption and how it changes bundled font loading behavior.partytowndependency to the newer package source/version.