feat: add next preset to webpack-dev-server-fresh #21069

ZachJW34 · 2022-04-13T17:16:23Z

Closes UNIFY-1400

User facing changelog

Add next support for new Object API in webpack-dev-server-fresh

Additional details

Mostly a lift and shift from npm/react/plugins for next specific webpack code. Since Next ships it's own version of webpack, I had to change how we source webpack deps to account for this. The most notable change is the removal of Next v10 support which is two major versions behind the current Next v12. I also changed how we detect if swc is being used. It was accounting for the possibility of being used on 9.x but we don't have that requirement for the new packages.

How has the user experience changed?

Screen.Recording.2022-04-13.at.12.13.54.PM.mov

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?
Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-04-13T17:16:25Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cypress · 2022-04-13T17:34:25Z

Test summary

17881 • 0 • 217 • 0 • 1

Run details


Project	cypress
Status	Passed
Commit	`c521e1c`
Started	Apr 19, 2022 10:14 PM
Ended	Apr 19, 2022 10:28 PM
Duration	14:13 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	cypress/e2e/commands/xhr.cy.js		1
1	... > no status when request isnt forced 404

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

ZachJW34 · 2022-04-13T23:11:49Z

Need to update the fixture with the new data-cy-root selector.

lmiller1990

Code seems fine and tests pass - I think some comments might be useful, considering how many different branches there are for the next.js handler.

Support for next@latest is awesome and a must have, great to see this 🚀

lmiller1990 · 2022-04-14T01:35:31Z

npm/webpack-dev-server-fresh/src/helpers/nextHandler.ts

+    return pagesDir
+  }
+
+  pagesDir = path.join(projectRoot, 'src/pages')


Should it be path.join(projectRoot, 'src', 'pages') to use the correct separator on windows?

lmiller1990 · 2022-04-14T01:36:31Z

npm/webpack-dev-server-fresh/src/helpers/sourceRelativeWebpackModules.ts

+    let webpack5 = true
+    const importPath = path.dirname(webpackJsonPath)
+    const webpackModule = require(path.join(importPath, 'webpack.js'))
+


This entire webpack module file is getting to be really complex really fast, I wonder if we can find some cleaner way to handle this eventually...

I agree, @tgriesser might have some good ideas on cleaning this up. I think we should allow the handlers to source the information so we don't have the framework bloat in this file, and sourceRelativeWebpackModules.ts could be helper functions that the handlers consume, but not going to refactor in this PR.

Agreed, this stuff is all super ugly. It makes me want to have a next-dev-server separate from our webpack-dev-server, since it's requiring so much custom configuration / patching.

Or at the very least its own file / function in webpack-dev-server. webpackConfig = framework === 'nextjs') ? getNextWebpackConfig() : getWebpackConfig(), and they can share utility functions as needed.

The more I look at it the more it feels like something we should decide now rather than later, pull it out in some fashion rather than kick it down the road.

I'm going to refactor this! I'm sidetracked getting the dev-servers to work with the built binary but going to return

Yeah I think the next stuff should be handled separately than the regular one.

The first attempt here was intentionally procedural, until we knew what we needed to reuse for next, it didn’t make sense to abstract.

Refactored, broke the functions up so they could be reused and altered the handler contract so that each handler is responsible for providing the sourced deps.

lmiller1990 · 2022-04-14T01:38:47Z

npm/webpack-dev-server-fresh/src/helpers/nextHandler.ts

+const debug = debugLib('cypress:webpack-dev-server-fresh:nextHandler')
+
+export async function nextHandler ({ devServerConfig, sourceWebpackModulesResult }: PresetHandler) {
+  const webpackConfig = await loadWebpackConfig({ devServerConfig, sourceWebpackModulesResult })


This look really thorough, nice job -- it's a little challenging to fully understand what case each branch handles, might be a good candidate for some fairly verbose comments?

Added some more comments throughout

…eset

BlueWinds · 2022-04-14T17:01:43Z

npm/webpack-dev-server-fresh/src/devServer.ts

@@ -137,6 +138,10 @@ devServer.create = async function (devServerConfig: WebpackDevServerConfig) {
      case 'vue-cli':
        frameworkConfig = vueCliHandler({ devServerConfig, sourceWebpackModulesResult })
        break
+


Just a style thing, with big switch statements like this, I always prefer having them out in their own function, so you can just return from each branch, a'la

getConfig = async (framework) => { switch (framework) { case 'react': return case 'nuxt': return await nuxtHandler({ devServerConfig, sourceWebpackModulesResult }) ...etc... } } let frameworkConfig = await getConfig(devServerConfig.framework)

Addressed in ac29f02

BlueWinds · 2022-04-14T17:11:08Z

npm/webpack-dev-server-fresh/src/helpers/nextHandler.ts

+}
+
+/**
+ * Acquire the modules needed to load the Next webpack config


I'm not really following this comment. Acquire them how, for what, why, from where?

Addressed in ac29f02

BlueWinds · 2022-04-14T17:17:51Z

npm/webpack-dev-server-fresh/src/helpers/sourceRelativeWebpackModules.ts

+    let webpack5 = true
+    const importPath = path.dirname(webpackJsonPath)
+    const webpackModule = require(path.join(importPath, 'webpack.js'))
+


Agreed, this stuff is all super ugly. It makes me want to have a next-dev-server separate from our webpack-dev-server, since it's requiring so much custom configuration / patching.

BlueWinds · 2022-04-14T17:19:59Z

npm/webpack-dev-server-fresh/src/helpers/sourceRelativeWebpackModules.ts

+    let webpack5 = true
+    const importPath = path.dirname(webpackJsonPath)
+    const webpackModule = require(path.join(importPath, 'webpack.js'))
+


Or at the very least its own file / function in webpack-dev-server. webpackConfig = framework === 'nextjs') ? getNextWebpackConfig() : getWebpackConfig(), and they can share utility functions as needed.

BlueWinds · 2022-04-14T17:21:35Z

npm/webpack-dev-server-fresh/src/helpers/sourceRelativeWebpackModules.ts

+    let webpack5 = true
+    const importPath = path.dirname(webpackJsonPath)
+    const webpackModule = require(path.join(importPath, 'webpack.js'))
+


The more I look at it the more it feels like something we should decide now rather than later, pull it out in some fashion rather than kick it down the road.

…eset

lmiller1990

One comment around unnecessary await.

Any other blocker? Let's get this merged CT Arch and this PR should be #1 priority.

lmiller1990 · 2022-04-19T05:43:27Z

npm/webpack-dev-server-fresh/src/devServer.ts

+      return await nuxtHandler(devServerConfig)
+
+    case 'vue-cli':
+      return await vueCliHandler(devServerConfig)


No need `await here.

npm/webpack-dev-server-fresh/src/devServer.ts

…e-config * 10.0-release: chore: Move component-index generation to scaffold-config package (#21090) fix: label text should be clickable to toggle snapshot highlight (#21122) feat: add next preset to webpack-dev-server-fresh (#21069) chore: add dev-servers as deps to server to be included in the binary (#21142) fix: do not highlight preExtension if selected option is renameFolder (#21121) fix(unify): Remove 'Reconfigure' dropdown from Testing Type chooser (#21063) feat(unify): launchpad header breadcrumbs and reusable tooltip component (#20648) test: add windows-test-kitchensink job (#20847) fix: aut centering and height calculation (#21019) chore: fix tests that fail on windows (#21055) fix: running a new test after already having run tests (#21087) fix: ct project setup framework keys for next and nuxt (#21116) fix: remove MountReturn from scaffolded ct support file (#21119) fix: do not scaffold fixtures if folder exist (#21078) fix: revert "fix: types for Cypress.Commands.add (#20376)" (#21104) chore: Update Chrome (stable) to 100.0.4896.127 and Chrome (beta) to 101.0.4951.34 (#21083) fix: types for Cypress.Commands.add (#20376) (#20377)

feat: add next preset to webpack-dev-server-fresh

d0c113f

ZachJW34 requested review from a team as code owners April 13, 2022 17:16

ZachJW34 requested review from jennifer-shehane, tgriesser, BlueWinds and lmiller1990 and removed request for a team April 13, 2022 17:16

jennifer-shehane removed their request for review April 13, 2022 18:02

ZachJW34 and others added 2 commits April 13, 2022 14:11

Merge branch '10.0-release' into zachw/next-preset

897961b

Merge branch '10.0-release' into zachw/next-preset

d860c8e

lmiller1990 approved these changes Apr 14, 2022

View reviewed changes

ZachJW34 added 2 commits April 14, 2022 09:16

Merge remote-tracking branch 'origin/10.0-release' into zachw/next-pr…

2d62850

…eset

add comments, address feedback and fix tests

51e60f7

BlueWinds suggested changes Apr 14, 2022

View reviewed changes

ZachJW34 added 2 commits April 15, 2022 15:20

Merge remote-tracking branch 'origin/10.0-release' into zachw/next-pr…

a65983d

…eset

refactor: sourceWebpack deps and handler config

ac29f02

ZachJW34 requested a review from BlueWinds April 18, 2022 15:04

lmiller1990 approved these changes Apr 19, 2022

View reviewed changes

tgriesser reviewed Apr 19, 2022

View reviewed changes

npm/webpack-dev-server-fresh/src/devServer.ts Outdated Show resolved Hide resolved

tgriesser added 2 commits April 19, 2022 09:12

Update npm/webpack-dev-server-fresh/src/devServer.ts

0d7209a

Merge branch '10.0-release' into zachw/next-preset

574d0b6

BlueWinds approved these changes Apr 19, 2022

View reviewed changes

Merge branch '10.0-release' into zachw/next-preset

c521e1c

ZachJW34 merged commit ccfee1e into 10.0-release Apr 19, 2022

ZachJW34 deleted the zachw/next-preset branch April 19, 2022 22:52

sync-by-unito bot mentioned this pull request May 31, 2022

Release: Ensure we are publishing the correct versions of CT related npm packages for upcoming 10.0 release #21944

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add next preset to webpack-dev-server-fresh #21069

feat: add next preset to webpack-dev-server-fresh #21069

ZachJW34 commented Apr 13, 2022

cypress-bot bot commented Apr 13, 2022

cypress bot commented Apr 13, 2022 •

edited

Loading

ZachJW34 commented Apr 13, 2022

lmiller1990 left a comment •

edited

Loading

lmiller1990 Apr 14, 2022

lmiller1990 Apr 14, 2022

ZachJW34 Apr 14, 2022

BlueWinds Apr 14, 2022

BlueWinds Apr 14, 2022

BlueWinds Apr 14, 2022

ZachJW34 Apr 14, 2022

tgriesser Apr 14, 2022

ZachJW34 Apr 18, 2022

lmiller1990 Apr 14, 2022

ZachJW34 Apr 14, 2022

BlueWinds Apr 14, 2022

ZachJW34 Apr 19, 2022

BlueWinds Apr 14, 2022

ZachJW34 Apr 19, 2022

BlueWinds Apr 14, 2022

BlueWinds Apr 14, 2022

BlueWinds Apr 14, 2022

lmiller1990 left a comment

lmiller1990 Apr 19, 2022

tgriesser Apr 19, 2022

feat: add next preset to webpack-dev-server-fresh #21069

feat: add next preset to webpack-dev-server-fresh #21069

Conversation

ZachJW34 commented Apr 13, 2022

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Apr 13, 2022

cypress bot commented Apr 13, 2022 • edited Loading

Test summary

Run details

Flakiness

ZachJW34 commented Apr 13, 2022

lmiller1990 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Apr 13, 2022 •

edited

Loading

lmiller1990 left a comment •

edited

Loading