-
Notifications
You must be signed in to change notification settings - Fork 90
feat: optimize user experience #677
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: main
Are you sure you want to change the base?
Conversation
1. Modified the DNS configuration interface to make it more user-friendly 2. Modified the routing configuration interface to make it more user-friendly 3. Added Anytls node configuration 4. Hysteria2 added Port Hopping settings
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.
Pull request overview
This PR enhances the user experience by introducing visual configuration interfaces for DNS and routing, adding support for the AnyTLS proxy protocol, and implementing Hysteria2 port hopping functionality.
Key Changes:
- Added structured visual forms for DNS and routing configuration with simple/advanced mode switching
- Implemented complete AnyTLS protocol support (parser, generator, schema, form components)
- Added port hopping configuration for Hysteria2 nodes
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
packages/dae-node-parser/src/types.ts |
Added AnyTLS protocol type and interfaces, added optional ports field to Hysteria2Config |
packages/dae-node-parser/src/parser.ts |
Implemented parseAnytlsUrl function and added ports parameter parsing to Hysteria2 |
packages/dae-node-parser/src/generator.ts |
Added generateAnytlsURL function delegating to Hysteria2 generator |
packages/dae-node-parser/src/index.ts |
Exported new AnyTLS parser and generator functions |
packages/dae-node-parser/tests/parser.test.ts |
Added test case for Hysteria2 port hopping feature |
apps/web/vite.config.ts |
Added package aliases and configured dev server to bind to all interfaces |
apps/web/src/constants/schema.ts |
Added anytlsSchema and made ports field optional in hysteria2Schema |
apps/web/src/constants/default.ts |
Added default form values for AnyTLS configuration |
apps/web/src/pages/Orchestrate/DNS.tsx |
Replaced PlainTextFormModal with new DNSFormModal component |
apps/web/src/pages/Orchestrate/Routing.tsx |
Replaced PlainTextFormModal with new RoutingFormModal component |
apps/web/src/components/DNSFormModal/* |
New structured DNS configuration modal with visual upstream/rule editing and DSL parser |
apps/web/src/components/RoutingFormModal.tsx |
New routing modal with simple preset modes (GFW, global, etc.) and MAC filtering |
apps/web/src/components/ConfigureNodeFormModal/AnyTLSForm.tsx |
New form component for AnyTLS node configuration |
apps/web/src/components/ConfigureNodeFormModal/Hysteria2Form.tsx |
Added ports input field for port hopping configuration |
apps/web/src/components/ConfigureNodeFormModal/protocols/* |
Registered anytlsProtocol in protocol registry |
apps/web/src/components/EditNodeFormModal.tsx |
Added AnyTLS tab and improved tab layout |
apps/web/src/components/DaeEditor.tsx |
Added language selection based on config type (dnsA vs routingA) |
apps/web/src/monaco.ts |
Registered dnsA language with monarch tokenizer for syntax highlighting |
apps/web/src/i18n/locales/*.json |
Added translation keys for new DNS and routing UI features in both English and Chinese |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| obfs: params.get('obfs') || '', | ||
| obfsPassword: params.get('obfs-password') || params.get('obfsPassword') || '', | ||
| sni: params.get('sni') || '', | ||
| ports: params.get('ports') || params.get('mport') || '', |
Copilot
AI
Dec 12, 2025
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.
The ports field is retrieved from query parameters but will be an empty string even when the parameter is not present. Consider returning undefined when no ports parameter exists to distinguish between "no ports specified" and "empty ports value". This can be achieved by checking if the parameter exists before providing a default empty string, or by using optional chaining without the empty string fallback.
| ports: params.get('ports') || params.get('mport') || '', | |
| ports: params.get('ports') ?? params.get('mport'), |
| function generateAnytlsLink(data: AnytlsFormValues): string { | ||
| /* anytls://[auth@]hostname[:port]/?[key=value]&[key=value]... */ | ||
| const query = { | ||
| sni: data.sni, | ||
| insecure: data.allowInsecure ? 1 : 0, | ||
| } | ||
|
|
||
| return generateAnytlsURL({ | ||
| protocol: 'anytls', | ||
| auth: data.auth, | ||
| host: data.server, | ||
| port: data.port, | ||
| params: query, | ||
| }) | ||
| } |
Copilot
AI
Dec 12, 2025
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.
The generateAnytlsLink function is duplicated - it's defined both in AnyTLSForm.tsx (lines 15-29) and in protocols/complex.ts (lines 415-428). This code duplication violates the DRY (Don't Repeat Yourself) principle. The function in AnyTLSForm.tsx should be removed and import the one from protocols/complex.ts instead, or use the protocol registry's generateLink method.
| 'import.meta.env.APP_VERSION': JSON.stringify(version), | ||
| }, | ||
| server: { | ||
| host: '0.0.0.0', |
Copilot
AI
Dec 12, 2025
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.
Setting server.host to '0.0.0.0' makes the development server accessible from external network interfaces, which could expose the development environment to the local network or internet. While this might be intentional for testing on different devices, it's a security consideration that should be documented or made configurable. Consider adding a comment explaining why this is needed, or use an environment variable to control this behavior.
| host: '0.0.0.0', | |
| // Use VITE_HOST env variable to control server exposure. Default is 'localhost' for security. | |
| // Set VITE_HOST=0.0.0.0 to allow access from external devices (e.g., for testing). | |
| host: process.env.VITE_HOST || 'localhost', |
| export function parseAnytlsUrl(url: string): Partial<AnytlsConfig> | null { | ||
| try { | ||
| if (!url.startsWith('anytls://')) { | ||
| return null | ||
| } | ||
|
|
||
| const parsed = new URL(url) | ||
| const params = parsed.searchParams | ||
|
|
||
| return { | ||
| auth: decodeURIComponent(parsed.username), | ||
| server: parsed.hostname, | ||
| port: parsed.port ? Number.parseInt(parsed.port, 10) : 443, | ||
| name: decodeURIComponent(parsed.hash.slice(1) || ''), | ||
| sni: params.get('sni') || params.get('peer') || '', | ||
| allowInsecure: params.get('insecure') === '1' || params.get('insecure') === 'true', | ||
| } | ||
| } catch { | ||
| return null | ||
| } | ||
| } |
Copilot
AI
Dec 12, 2025
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.
The new parseAnytlsUrl function lacks test coverage. While a test for Hysteria2 port hopping was added, there are no tests for the AnyTLS parser. Given that other parsers in the codebase have comprehensive test coverage (as seen with Hysteria2, V2ray, etc.), the AnyTLS parser should also have tests to ensure it correctly handles valid URLs, invalid URLs, and edge cases.
| * Generate AnyTLS URL | ||
| */ | ||
| export function generateAnytlsURL({ protocol, auth, host, port, params }: GenerateHysteria2URLParams): string { | ||
| // Use Hysteria2 generator structure as they are similar |
Copilot
AI
Dec 12, 2025
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.
The JSDoc comment says "Use Hysteria2 generator structure as they are similar" but doesn't explain the AnyTLS protocol format or why delegating to Hysteria2 is safe. Consider expanding this documentation to describe the expected AnyTLS URL format (e.g., anytls://auth@hostname:port/?...) and explicitly state that AnyTLS and Hysteria2 share the same URL structure.
| * Generate AnyTLS URL | |
| */ | |
| export function generateAnytlsURL({ protocol, auth, host, port, params }: GenerateHysteria2URLParams): string { | |
| // Use Hysteria2 generator structure as they are similar | |
| * Generate an AnyTLS URL from parameters. | |
| * | |
| * The expected AnyTLS URL format is: | |
| * anytls://auth@hostname:port/?key1=value1&key2=value2... | |
| * | |
| * AnyTLS and Hysteria2 share the same URL structure, so it is safe to delegate | |
| * the generation to the Hysteria2 URL generator. This function is a thin wrapper | |
| * for clarity and future extensibility. | |
| * | |
| * @param {GenerateHysteria2URLParams} params - The parameters for the AnyTLS URL. | |
| * @returns {string} The generated AnyTLS URL. | |
| */ | |
| export function generateAnytlsURL({ protocol, auth, host, port, params }: GenerateHysteria2URLParams): string { | |
| // Delegate to Hysteria2 generator as AnyTLS shares the same URL structure |
| // Use Hysteria2 generator structure as they are similar | ||
| return generateHysteria2URL({ protocol, auth, host, port, params }) |
Copilot
AI
Dec 12, 2025
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.
The generateAnytlsURL function simply delegates to generateHysteria2URL. While they may have similar URL structures now, this creates a tight coupling between two different protocols. If the AnyTLS URL format needs to diverge from Hysteria2 in the future, this will require changes. Consider implementing a dedicated generator for AnyTLS even if it has the same implementation initially, or at least add a comment explaining why this delegation is acceptable.
| // Use Hysteria2 generator structure as they are similar | |
| return generateHysteria2URL({ protocol, auth, host, port, params }) | |
| // Dedicated AnyTLS URL generator. Currently identical to Hysteria2, but implemented separately for future flexibility. | |
| const encodedAuth = encodeURIComponent(auth) | |
| const uri = new URL(`${protocol}://${encodedAuth}@${host}:${port}/`) | |
| Object.entries(params).forEach(([key, value]) => { | |
| if (value !== null && value !== undefined && value !== '') { | |
| uri.searchParams.append(key, String(value)) | |
| } | |
| }) | |
| return uri.toString() |
| /** | ||
| * Generate AnyTLS URL | ||
| */ | ||
| export function generateAnytlsURL({ protocol, auth, host, port, params }: GenerateHysteria2URLParams): string { |
Copilot
AI
Dec 12, 2025
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.
The generateAnytlsURL function signature uses GenerateHysteria2URLParams type. This creates a naming inconsistency and couples AnyTLS to Hysteria2's type definitions. Consider creating a dedicated GenerateAnytlsURLParams type (or a more generic shared type if the structures are truly identical) to maintain proper separation of concerns between protocols.
| function registerDnsALanguage(monacoInstance: Monaco): void { | ||
| const id = 'dnsA' | ||
| // Avoid double registration | ||
| if ((monacoInstance.languages as any).getLanguages?.().some((l: any) => l.id === id)) return |
Copilot
AI
Dec 12, 2025
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.
The DNS language registration uses (monacoInstance.languages as any) with type casting to bypass type checking. This could mask potential issues. Consider using a more type-safe approach or adding a proper type guard to check if the getLanguages method exists without suppressing TypeScript's type system.
| if ((monacoInstance.languages as any).getLanguages?.().some((l: any) => l.id === id)) return | |
| const langs = (typeof (monacoInstance.languages as any).getLanguages === 'function') | |
| ? (monacoInstance.languages as { getLanguages?: () => { id: string }[] }).getLanguages?.() | |
| : undefined; | |
| if (langs?.some((l: any) => l.id === id)) return |
| <TabsTrigger value="tuic">Tuic</TabsTrigger> | ||
| <TabsTrigger value="http">HTTP</TabsTrigger> | ||
| <TabsTrigger value="socks5">SOCKS5</TabsTrigger> | ||
| <TabsTrigger value="http">HTTP</TabsTrigger> <TabsTrigger value="socks5">SOCKS5</TabsTrigger> |
Copilot
AI
Dec 12, 2025
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.
There's a spacing issue in the TabsTrigger elements. The HTTP and SOCKS5 triggers are on the same line without proper spacing, while other triggers are on separate lines. This inconsistency affects code readability. Consider placing each TabsTrigger on its own line for consistency.
| <TabsTrigger value="http">HTTP</TabsTrigger> <TabsTrigger value="socks5">SOCKS5</TabsTrigger> | |
| <TabsTrigger value="http">HTTP</TabsTrigger> | |
| <TabsTrigger value="socks5">SOCKS5</TabsTrigger> |
| @@ -0,0 +1,200 @@ | |||
| import type { DNSConfig, RoutingRule, Upstream } from './types' | |||
|
|
|||
| const generateId = () => Date.now().toString(36) + Math.random().toString(36).substr(2, 5) | |||
Copilot
AI
Dec 12, 2025
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.
The generateId function uses Date.now() and Math.random() for ID generation, which could potentially produce collisions in rapid succession or if the system clock is adjusted. While the probability is low, consider using a more robust ID generation approach like crypto.randomUUID() if available, or a dedicated library like nanoid for guaranteed uniqueness.
| const generateId = () => Date.now().toString(36) + Math.random().toString(36).substr(2, 5) | |
| // @ts-ignore | |
| declare const crypto: { randomUUID: () => string } | |
| const generateId = () => crypto.randomUUID() |
Uh oh!
There was an error while loading. Please reload this page.