-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: multiple proxy settting support #3228
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
base: master
Are you sure you want to change the base?
Conversation
|
Do you want me to review this PR? Please comment |
|
/review |
|
PR Summary: Adds support for multiple proxy entries in configuration and updates runtime handling to return an array of proxy mappings.
|
|
Reviewed up to commit:ad8a1c4489fe37a0566831d0d5e256cc03e44c2b Additional Suggestiondocs/settings.md, line:99-140Documentation still shows the old single-object proxy example (see lines 99-140 in docs/settings.md). Update docs to reflect the new array format (example config and explanation) and clearly highlight this breaking change so users can migrate their settings.* **`liveServer.settings.proxy`:** To enable proxy.
* *Properties :*
```js
/*
You can configure multiple proxy rules. Each item maps a baseUri
(what Live Server serves) to a target proxyUri (your real backend).
*/
"liveServer.settings.proxy": [
{
"enable": true, // set it true to enable this proxy rule
"baseUri": "/api", // from where you want to proxy
"proxyUri": "http://localhost:3000/" // the actual url
},
{
"enable": false,
"baseUri": "/php",
"proxyUri": "http://localhost/php/"
}
],
```
> **Breaking change:** `liveServer.settings.proxy` used to be a single object.
> It is now an array of proxy definitions. To migrate, wrap your existing
> proxy object in an array, for example:
>
> ```js
> // Before
> "liveServer.settings.proxy": {
> "enable": true,
> "baseUri": "/",
> "proxyUri": "http://localhost/php/"
> }
>
> // After
> "liveServer.settings.proxy": [
> {
> "enable": true,
> "baseUri": "/",
> "proxyUri": "http://localhost/php/"
> }
> ]
> ```Others- This PR introduces a breaking change (settings.proxy becomes an array). Ensure release notes and changelog explicitly mention the migration, and consider maintaining graceful handling for the old object format (either at Config.getProxy as suggested above or during startup) so existing users are not immediately broken.// src/Config.ts
public static get getProxy(): IProxy[] {
const value = Config.getSettings<IProxy | IProxy[]>('proxy');
// Backward compatibility: old single-object config
if (value && !Array.isArray(value)) {
return [value];
}
return value || [];
}
// Additionally, on extension activation (not shown in diff),
// consider logging a deprecation/migration notice if the old
// object-shaped configuration is detected.Few more points:
"liveServer.settings.proxy": { Then verify the schema with VSCode (reload) to ensure the settings UI and validation behave correctly. "liveServer.settings.proxy": {
"type": "array",
"default": [
{
"enable": false,
"baseUri": "/",
"proxyUri": "http://127.0.0.1:80"
}
],
"items": {
"type": "object",
"properties": {
"enable": {
"type": "boolean",
"default": false,
"description": "Make it true to enable the feature."
},
"baseUri": {
"type": "string",
"default": "/",
"pattern": ""
},
"proxyUri": {
"type": "string",
"default": "http://127.0.0.1:80",
"pattern": "(^http[s]?://)(.[^(\\|\\s)]+)$"
}
},
"required": [
"enable",
"baseUri",
"proxyUri"
],
"additionalProperties": false
},
"description": "To Setup Proxy"
} |
|
/review |
| "type": "array", | ||
| "default": [ | ||
| { | ||
| "enable": false, | ||
| "baseUri": "/", | ||
| "proxyUri": "http://127.0.0.1:80" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "enable", | ||
| "baseUri", | ||
| "proxyUri" | ||
| ], | ||
| "additionalProperties": false, | ||
| "items": { | ||
| "type": "object", | ||
| "properties": { | ||
| "enable": { | ||
| "type": "boolean", | ||
| "default": false, | ||
| "description": "Make it true to enable the feature." | ||
| }, | ||
| "baseUri": { | ||
| "type": "string", | ||
| "default": "/", | ||
| "pattern": "" | ||
| }, | ||
| "proxyUri": { | ||
| "type": "string", | ||
| "default": "http://127.0.0.1:80", | ||
| "pattern": "(^http[s]?://)(.[^(\\|\\s)]+)$" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "enable", | ||
| "baseUri", | ||
| "proxyUri" | ||
| ], | ||
| "additionalProperties": false |
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.
[NITPICK] You changed the settings schema from an object to an array (breaking). Update any user-facing documentation and examples to reflect the new array form (docs/settings.md still shows the old single-object example). Also add a clear migration note in README/CHANGELOG describing the breaking change and example of the new shape so users know how to update their settings.
### Breaking change: `liveServer.settings.proxy`
The `liveServer.settings.proxy` configuration has changed from a single object to an array of proxy definitions.
**Before (single proxy object):**
```jsonc
"liveServer.settings.proxy": {
"enable": true,
"baseUri": "/api",
"proxyUri": "http://127.0.0.1:3000"
}After (multiple proxy objects):
"liveServer.settings.proxy": [
{
"enable": true,
"baseUri": "/api",
"proxyUri": "http://127.0.0.1:3000"
},
{
"enable": true,
"baseUri": "/auth",
"proxyUri": "http://127.0.0.1:4000"
}
]Existing users must wrap their previous single proxy object in an array to avoid breakage.
Also update docs/settings.md to show the new array format under liveServer.settings.proxy.
| public static get getProxy(): IProxy[] { | ||
| const val = Config.getSettings<IProxy | IProxy[]>('proxy'); | ||
| if (!val) { | ||
| return []; | ||
| } | ||
| return Array.isArray(val) ? val : [val]; |
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.
[VALIDATION] getProxy now returns IProxy[] and wraps single-object values into an array which is good for backward compatibility. However, you're trusting the value from configuration without validating its shape. Add a runtime guard that validates each array element (ensure enable is boolean, baseUri and proxyUri are strings and proxyUri matches the expected protocol pattern) before returning it. This prevents later code from throwing if users set an invalid custom config.
public static get getProxy(): IProxy[] {
const raw = Config.getSettings<unknown>('proxy');
if (!raw) {
return [];
}
const toArray = Array.isArray(raw) ? raw : [raw];
const proxies: IProxy[] = [];
for (const item of toArray) {
if (!item || typeof item !== 'object') {
continue;
}
const { enable, baseUri, proxyUri } = item as Partial<IProxy>;
if (typeof enable !== 'boolean') {
continue;
}
if (typeof baseUri !== 'string') {
continue;
}
if (typeof proxyUri !== 'string') {
continue;
}
if (!/^https?:\/\/.+$/i.test(proxyUri)) {
continue;
}
proxies.push({ enable, baseUri, proxyUri } as IProxy);
}
return proxies;
}| static getProxySetup() { | ||
| const proxySetup = Config.getProxy; | ||
| let proxy = [[]]; | ||
| if (proxySetup.enable === true) { | ||
| proxy[0].push(proxySetup.baseUri, proxySetup.proxyUri); | ||
| let proxySetup = Config.getProxy; | ||
|
|
||
| // Handle missing/undefined config | ||
| if (!proxySetup) { | ||
| return null; | ||
| } | ||
| else { | ||
| proxy = null; // required to change the type [[]] to black array []. | ||
|
|
||
| // Backward compatibility: old single-object config | ||
| if (!Array.isArray(proxySetup)) { | ||
| proxySetup = [proxySetup]; | ||
| } | ||
|
|
||
| const validProxy = proxySetup.filter(item => item && typeof item === 'object' && item.enable); | ||
|
|
||
| if (!validProxy.length) { | ||
| return null; | ||
| } | ||
|
|
||
| const proxy = validProxy.map(item => { | ||
| return [item.baseUri, item.proxyUri]; | ||
| }); | ||
|
|
||
| return proxy; |
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.
[REFACTORING] This compatibility logic can be simplified and made safer: Config.getProxy already returns an array (empty if missing). Remove the redundant if (!proxySetup) return null; and the !Array.isArray branch (it will never run with current Config.getProxy). Also validate/normalize each proxy entry before mapping (ensure baseUri starts with '/' or normalize it, ensure proxyUri is a valid http(s) URL). Finally, handle the case of duplicate or overlapping baseUri rules (either document behavior or deduplicate) so consumers of the returned array won't get unexpected routing conflicts.
static getProxySetup() {
const proxies = Config.getProxy; // always an array
const validProxy = proxies
.filter((item) => item && typeof item === 'object' && item.enable)
.map((item) => {
let baseUri = item.baseUri || '/';
if (!baseUri.startsWith('/')) {
baseUri = `/${baseUri}`;
}
// Basic normalization: trim spaces
const proxyUri = (item.proxyUri || '').trim();
return { ...item, baseUri, proxyUri };
});
if (!validProxy.length) {
return null;
}
// Optional: dedupe by baseUri, keep first occurrence
const seen = new Set<string>();
const deduped = validProxy.filter((item) => {
if (seen.has(item.baseUri)) {
return false;
}
seen.add(item.baseUri);
return true;
});
return deduped.map((item) => [item.baseUri, item.proxyUri]);
}|
Reviewed up to commit:eb4dc3dbdf5550da2ecc28709822ce6f6b5e222c Additional SuggestionOthers- Breaking-change impact: you changed the runtime shape of `liveServer.settings.proxy` in package.json to an array. Ensure every other consumer in the codebase (not only Helper.getProxySetup) that reads Config.getProxy has been updated to accept an array. If any other module still expects a single object, runtime errors will occur. Run a global search for usages of `getProxy`/`liveServer.settings.proxy` and update them to handle arrays or ensure Config.getProxy covers all conversions consistently. |
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Only Support sigle proxy setting
Issue Number: #2524
What is the new behavior?
Make modification for supporting multiple proxy setting
Does this PR introduce a breaking change?
liveServer.settings.proxyshould change to array from object, example,{ "liveServer.settings.proxy": [ { "enable": false, "baseUri": "/", "proxyUri": "http://127.0.0.1:80" } ] }Other information
N/A