fix: allow cypress config.port to override devServer.port for proxying assets #27677
Conversation
| @@ -122,6 +125,7 @@ function makeCypressViteConfig (config: ViteDevServerConfig, vite: Vite): Inline | |||
| vite.searchForWorkspaceRoot?.(process.cwd()), | |||
| ], | |||
| }, | |||
| port: vitePort, | |||
There was a problem hiding this comment.
Could just be port - if port is provided, it's a number, otherwise it'll be undefined. No need for the vitePort variable
There was a problem hiding this comment.
No it’s needed because of types. Cypress provided types are number or null. Vite is number or undefined. Switch it and it’ll yell at you.
| @@ -106,6 +109,7 @@ export function makeCypressWebpackConfig ( | |||
| return { | |||
| ...finalConfig, | |||
| devServer: { | |||
| port: webpackDevServerPort, | |||
There was a problem hiding this comment.
Again just port is okay, but this is more explicit
There was a problem hiding this comment.
Types problem without coercing to undefined
| @@ -240,6 +240,11 @@ export class ProjectLifecycleManager { | |||
|
|
|||
| const devServerOptions = await this.ctx._apis.projectApi.getDevServer().start({ specs: this.ctx.project.specs, config: finalConfig }) | |||
|
|
|||
| // if we received a cypressConfig.port we want to null it out | |||
| // because we propagated it into the devServer.port and it is | |||
| // later set as baseUrl which cypress is launched into | |||
There was a problem hiding this comment.
May be worth adding a link to the issue that explain this
system-tests/lib/normalizeStdout.ts
Outdated
| @@ -114,7 +115,7 @@ export const normalizeStdout = function (str: string, options: any = {}) { | |||
|
|
|||
| // remove all of the dynamic parts of stdout | |||
| // to normalize against what we expected | |||
| str = str | |||
| str = stripAnsi(str) | |||
There was a problem hiding this comment.
How come we need to change this to stripAnsi here
….com/cypress-io/cypress into fix/config-port-overriding-devserver
|
Would a user ever have another service running that is expecting the |
Co-authored-by: Bill Glesias <bglesias@gmail.com>
1 flaky test on run #50611 ↗︎
Details:
|
|||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs |
Test Replay
Output
Screenshots
|
|
Review all test suite changes for PR #27677 ↗︎
Just to clarify are you suggesting we check if the |
….com/cypress-io/cypress into fix/config-port-overriding-devserver
….com/cypress-io/cypress into fix/config-port-overriding-devserver
| export default defineConfig({ | ||
| videoCompression: false, // turn off video compression for CI |
There was a problem hiding this comment.
Probably not needed since it's off by default.
| export default defineConfig({ | |
| videoCompression: false, // turn off video compression for CI | |
| export default defineConfig({ |
Additional details
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?