feat: add reverse-proxy-prefix option to build#616
Merged
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:
WalkthroughAdds build-time reverse proxy prefix support: a parser that validates/normalizes a host/subpath prefix, computes an assetsDir and a /assets/* dynamic route, updates scoped config.yaml (serving.reverseProxyPrefix and dynamicRoutes) and scoped vite.config.js (build.assetsDir), extends ProjectStructure to resolve scoped/unscoped vite config paths, adds tests, and wires a --reverse-proxy-prefix CLI option that triggers applyReverseProxyOverride during build. Sequence Diagram(s)sequenceDiagram
participant CLI
participant Handler
participant Apply as applyReverseProxyOverride
participant Config as updateConfigYaml
participant ViteCfg as updateViteConfig
participant Vite as ViteBuild
CLI->>Handler: invoke build with reverseProxyPrefix
Handler->>Apply: call applyReverseProxyOverride(scope, prefix)
Apply->>Config: update config.yaml (serving.reverseProxyPrefix + dynamicRoutes)
Apply->>ViteCfg: update vite.config.js (build.assetsDir)
Handler->>Vite: run Vite build (after override applied)
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/pages/src/util/reverseProxyBuildOverride.test.ts (1)
12-48: ⚡ Quick winAdd parser regression tests for protocol/trailing-slash/invalid-character inputs.
Coverage is solid, but the parser should also be pinned for common edge inputs that directly affect both YAML and Vite rewrites.
Suggested additions
describe("parseReverseProxyBuildOverride", () => { + it("accepts protocol-prefixed values and derives the same subpath outputs", () => { + expect(parseReverseProxyBuildOverride("https://www.brand.com/locations")).toEqual({ + reverseProxyPrefix: "https://www.brand.com/locations", + assetsDir: "locations/assets", + dynamicRoute: { + from: "/assets/*", + to: "/locations/assets/:splat", + status: 200, + }, + }); + }); + + it("normalizes trailing slashes in subpath", () => { + expect(parseReverseProxyBuildOverride("www.brand.com/locations/")).toEqual({ + reverseProxyPrefix: "www.brand.com/locations/", + assetsDir: "locations/assets", + dynamicRoute: { + from: "/assets/*", + to: "/locations/assets/:splat", + status: 200, + }, + }); + }); + + it("throws when subpath contains unsupported characters", () => { + expect(() => parseReverseProxyBuildOverride('www.brand.com/loca"tions')).toThrow(); + });🤖 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/pages/src/util/reverseProxyBuildOverride.test.ts` around lines 12 - 48, The test suite for parseReverseProxyBuildOverride is missing regression cases for inputs containing URL protocols, trailing slashes, and invalid characters; add unit tests in reverseProxyBuildOverride.test.ts that call parseReverseProxyBuildOverride with strings like "https://www.brand.com/foo", "www.brand.com/foo/", and inputs containing illegal characters and assert the expected behavior (either specific thrown error messages or normalized outputs) so the parser is pinned for YAML/Vite rewrite consumers; reference the existing tests for structure and assert messages matching the parser's validation (use parseReverseProxyBuildOverride as the target function and check for throws on protocol/trailing-slash/invalid-character cases).
🤖 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/pages/src/util/reverseProxyBuildOverride.ts`:
- Around line 24-43: The current parsing of reverseProxyPrefix incorrectly
treats full URLs and unsafe characters as the subpath; update the parsing logic
that derives subpath (used to compute assetsDir and dynamicRoute.to) to first
detect and strip any URL scheme/host (e.g., if reverseProxyPrefix contains "://"
or looks like a host/path use URL parsing), then normalize the path by removing
leading/trailing slashes and collapsing duplicate slashes, validate/sanitize
characters (restrict to safe characters like alphanumerics, -, _, and / or
percent-decode then validate) and throw a clear Error if the resulting subpath
is empty or contains invalid chars; finally use that normalized subpath when
creating assetsDir and dynamicRoute.to/from so assetsDir and the routing targets
are safe and correct.
---
Nitpick comments:
In `@packages/pages/src/util/reverseProxyBuildOverride.test.ts`:
- Around line 12-48: The test suite for parseReverseProxyBuildOverride is
missing regression cases for inputs containing URL protocols, trailing slashes,
and invalid characters; add unit tests in reverseProxyBuildOverride.test.ts that
call parseReverseProxyBuildOverride with strings like
"https://www.brand.com/foo", "www.brand.com/foo/", and inputs containing illegal
characters and assert the expected behavior (either specific thrown error
messages or normalized outputs) so the parser is pinned for YAML/Vite rewrite
consumers; reference the existing tests for structure and assert messages
matching the parser's validation (use parseReverseProxyBuildOverride as the
target function and check for throws on
protocol/trailing-slash/invalid-character cases).
🪄 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: ffc6c3bf-247f-4059-9468-9860485ec86a
📒 Files selected for processing (4)
packages/pages/src/build/build.tspackages/pages/src/prod/prod.tspackages/pages/src/util/reverseProxyBuildOverride.test.tspackages/pages/src/util/reverseProxyBuildOverride.ts
jwartofsky-yext
changed the title
jwartofsky-yext
requested review from
asanehisa,
benlife5,
briantstephan and
mkilpatrick
benlife5
reviewed
Jun 9, 2026
mkilpatrick
reviewed
Jun 9, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/pages/src/util/reverseProxyOverride.ts (1)
75-77:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReturn the trimmed prefix for consistency.
The function returns the original
reverseProxyPrefix(with potential leading/trailing whitespace) whileassetsDiranddynamicRouteare computed from the trimmed version. This inconsistency could cause mismatches between the storedserving.reverseProxyPrefixin config.yaml and the actual asset paths.Proposed fix
return { - reverseProxyPrefix, + reverseProxyPrefix: trimmedReverseProxyPrefix, assetsDir: `${subpath}/assets`,🤖 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/pages/src/util/reverseProxyOverride.ts` around lines 75 - 77, The returned reverseProxyPrefix value is not trimmed while assetsDir and dynamicRoute are derived from the trimmed subpath; update the return to use the trimmed prefix (the same value used to compute subpath/dynamicRoute) so serving.reverseProxyPrefix matches asset paths — locate the reverseProxyOverride function and replace the untrimmed reverseProxyPrefix in the returned object with the trimmed variable (the same one used when computing assetsDir and dynamicRoute) to ensure consistency.
🤖 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/pages/src/util/reverseProxyOverride.test.ts`:
- Line 12: Update the test suite description to match the actual function under
test: change the describe block currently named "parseReverseProxyOverride" so
it references buildReverseProxyOverride (the function being tested) to avoid
confusion; locate the describe(...) that wraps tests for
buildReverseProxyOverride in reverseProxyOverride.test.ts and rename its string
to "buildReverseProxyOverride".
---
Duplicate comments:
In `@packages/pages/src/util/reverseProxyOverride.ts`:
- Around line 75-77: The returned reverseProxyPrefix value is not trimmed while
assetsDir and dynamicRoute are derived from the trimmed subpath; update the
return to use the trimmed prefix (the same value used to compute
subpath/dynamicRoute) so serving.reverseProxyPrefix matches asset paths — locate
the reverseProxyOverride function and replace the untrimmed reverseProxyPrefix
in the returned object with the trimmed variable (the same one used when
computing assetsDir and dynamicRoute) to ensure consistency.
🪄 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: 4cb49c42-9fb9-48c9-bfbd-eba3661a46e2
📒 Files selected for processing (6)
packages/pages/src/build/build.tspackages/pages/src/common/src/project/paths.tspackages/pages/src/common/src/project/structure.tspackages/pages/src/util/reverseProxyOverride.test.tspackages/pages/src/util/reverseProxyOverride.tspackages/pages/src/util/viteConfig.ts
✅ Files skipped from review due to trivial changes (1)
- packages/pages/src/common/src/project/paths.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/pages/src/build/build.ts
mkilpatrick
reviewed
Jun 9, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/pages/src/util/reverseProxyOverride.ts (1)
87-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the same Vite config resolver as the build command.
This helper always targets
vite.config.js, but the build path later comes fromscopedViteConfigPath(scope), and the PR objective/example repo usesvite.config.ts. In that case,--reverse-proxy-prefixwill fail here even though the build has a valid Vite config.🤖 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/pages/src/util/reverseProxyOverride.ts` around lines 87 - 95, The existence check currently hardcodes "vite.config.js" via viteConfigPath, which can miss other resolved configs (e.g. vite.config.ts); change the logic to use the same resolver used by the build by calling scopedViteConfigPath(scope) (the same symbol used later) to compute the Vite config path and check that file exists before applying reverseProxyPrefix, leaving configYamlPath checks as-is and updating the thrown error message to reference the resolved path variable.
🤖 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/pages/src/util/reverseProxyOverride.ts`:
- Around line 21-23: The function buildReverseProxyOverride currently trims
input into trimmedReverseProxyPrefix but later persists the original
reverseProxyPrefix into the config; update the persistence to use
trimmedReverseProxyPrefix instead so the written serving.reverseProxyPrefix
matches derived values (subpath, assetsDir, dynamic route). Locate
buildReverseProxyOverride and replace any uses that write reverseProxyPrefix
back to config.yaml (the save/persist step) with trimmedReverseProxyPrefix,
ensuring all derived fields (subpath, assetsDir, dynamic route) and the
persisted serving.reverseProxyPrefix are computed from the same trimmed value.
---
Outside diff comments:
In `@packages/pages/src/util/reverseProxyOverride.ts`:
- Around line 87-95: The existence check currently hardcodes "vite.config.js"
via viteConfigPath, which can miss other resolved configs (e.g. vite.config.ts);
change the logic to use the same resolver used by the build by calling
scopedViteConfigPath(scope) (the same symbol used later) to compute the Vite
config path and check that file exists before applying reverseProxyPrefix,
leaving configYamlPath checks as-is and updating the thrown error message to
reference the resolved path variable.
🪄 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: c7fc3ce5-0a02-4227-83cc-0a55e06e37dd
📒 Files selected for processing (2)
packages/pages/src/util/reverseProxyOverride.test.tspackages/pages/src/util/reverseProxyOverride.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/pages/src/util/reverseProxyOverride.test.ts
mkilpatrick
reviewed
Jun 9, 2026
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/pages/src/common/src/project/structure.ts`:
- Around line 221-224: The init() logic currently falls back to root
vite.config.js when a scoped file is missing, but getViteConfigPath() always
returns the scoped path; to make behavior consistent fail fast: in init() (where
viteConfigPath is computed) detect when config.scope is set and the resolved
scoped path (pathLib.resolve(config.scope ?? "", config.rootFiles.viteConfig))
does not exist, and throw/return an error instead of falling back to root;
update any callers expecting init() to succeed to handle the new error and keep
getViteConfigPath() unchanged (references: init(), getViteConfigPath(),
viteConfigPath, config.scope, config.rootFiles.viteConfig,
applyReverseProxyOverride).
🪄 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: 8613bc80-675a-42ed-8c80-bc2303b5530c
📒 Files selected for processing (4)
packages/pages/src/build/build.tspackages/pages/src/common/src/project/structure.tspackages/pages/src/util/reverseProxyOverride.test.tspackages/pages/src/util/reverseProxyOverride.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/pages/src/build/build.ts
- packages/pages/src/util/reverseProxyOverride.ts
- packages/pages/src/util/reverseProxyOverride.test.ts
mkilpatrick
reviewed
Jun 10, 2026
mkilpatrick
approved these changes
Jun 10, 2026
asanehisa
reviewed
Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After running "npm run build -- --reverse-proxy-prefix=www.brand.com/locations", I see these files modified:
vite.config.ts
config.yaml
The dist/assets are also updated:
See example of a run here: YextSolutions/pages-visual-editor-starter#330