-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
feat: support webpack-dev-server v5 in @cypress/webpack-dev-server #29306
Conversation
The changes in the network package have to do with types being installed in |
Passing run #54994 ↗︎
Details:
Review all test suite changes for PR #29306 ↗︎ |
@@ -180,11 +187,10 @@ export function sourceWebpack (config: WebpackDevServerConfig, framework: Source | |||
|
|||
// Source the webpack-dev-server module from the provided framework or projectRoot. | |||
// If none is found, we fallback to the version bundled with this package. | |||
export function sourceWebpackDevServer (config: WebpackDevServerConfig, framework?: SourcedDependency | null): SourcedWebpackDevServer { | |||
export function sourceWebpackDevServer (config: WebpackDevServerConfig, webpackMajorVersion: 4 | 5, framework?: SourcedDependency | null): SourcedWebpackDevServer { |
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.
Is 3 the version that is the fallback and why it isn't part of this webpackMajorVersion
union here?
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.
luckily for us we don't support webpack v3, only v4 and v5. But with this PR we would support webpack-dev-server v3, v4, and v5.
the package currently ships with webpack-dev-server v4 but will source whatever the user has installed
@AtofStryker Should we regard this as closed when this goes in? Or do we need some extra tests to verify on top of this? #29309 |
we need to officially support Angular 18 but this gets us a step closer. I was talking with @jordanpowell88 about this today and sounds like the scope of work isn't massive? i.e. we need some tests on top of it. The ones I know about are the system tests |
@AtofStryker This should also resolve these github actions failing outright: |
yes that it should! |
npm/webpack-dev-server/src/helpers/sourceRelativeWebpackModules.ts
Outdated
Show resolved
Hide resolved
/** Allows us to strip internal types sourced from webpack */ | ||
"stripInternal": true, | ||
"importsNotUsedAsValues": "error" |
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.
Why was this removed?
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.
I was getting deprecation warnings and I think I forgot to migrate it to the new flag https://www.typescriptlang.org/tsconfig#preserveValueImports. Ill try adding it back in and see whats up
@@ -42,7 +42,8 @@ | |||
"ts-node": "^10.9.2", | |||
"webpack": "npm:webpack@^5", | |||
"webpack-4": "npm:webpack@^4", | |||
"webpack-dev-server-3": "npm:webpack-dev-server@^3" | |||
"webpack-dev-server-3": "npm:webpack-dev-server@^3", | |||
"webpack-dev-server-5": "npm:webpack-dev-server@^5" |
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.
Out of curiosity, why isn't npm:webpack-dev-server@v4
listed?
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.
because its installed under webpack-dev-server
since its the default that ships with @cypress/webpack-dev-server
. Eventually we want to bump it to 5, but that is a breaking change
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.
Added a few comments but looks good!
…s.ts Co-authored-by: Matt Schile <mschile@cypress.io>
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
This PR introduces support for webpack-dev-server v5 inside @cypress/webpack-dev-server.
To avoid a breaking change, @cypress/webpack-dev-server still ships with wds v4 by default. This PR allows v5 to be discovered and supported if a user has wds v5 installed in their project.
The configuration is almost identical to that of wds v4, but I elected to keep both separate as feature configuration could evolve with v5 and not with v4, allowing us flexibility by keeping these options separate.
I have also added some system tests to verify things work as expected with wds5 and webpack 5. wds 5 does not work with webpack 4, so I added a custom error message inside of @cypress/webpack-dev-server to alert the user if they are using wds5 and webpack 4, as the error thrown under the hood is not graceful nor obvious.
I have also added this as a
feat
for cypress as this@cypress/webpack-dev-server
does ship with cypress by default.Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?