-
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: add next preset to webpack-dev-server-fresh #21069
Changes from 3 commits
d0c113f
897961b
d860c8e
2d62850
51e60f7
a65983d
ac29f02
0d7209a
574d0b6
c521e1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
// <reference types="cypress" /> | ||
/// <reference path="../support/e2e.ts" /> | ||
import type { ProjectFixtureDir } from '@tooling/system-tests/lib/fixtureDirs' | ||
|
||
const WEBPACK_REACT: ProjectFixtureDir[] = ['next-11', 'next-12', 'next-11-webpack-4'] | ||
|
||
// Add to this list to focus on a particular permutation | ||
const ONLY_PROJECTS: ProjectFixtureDir[] = [] | ||
|
||
for (const project of WEBPACK_REACT) { | ||
if (ONLY_PROJECTS.length && !ONLY_PROJECTS.includes(project)) { | ||
continue | ||
} | ||
|
||
describe(`Working with ${project}`, () => { | ||
beforeEach(() => { | ||
cy.scaffoldProject(project) | ||
cy.openProject(project) | ||
cy.startAppServer('component') | ||
}) | ||
|
||
it('should mount a passing test', () => { | ||
cy.visitApp() | ||
cy.contains('index.cy.js').click() | ||
cy.get('.passed > .num').should('contain', 1) | ||
}) | ||
|
||
it('should live-reload on src changes', () => { | ||
cy.visitApp() | ||
|
||
cy.contains('index.cy.js').click() | ||
cy.get('.passed > .num').should('contain', 1) | ||
|
||
cy.withCtx(async (ctx) => { | ||
const indexPath = ctx.path.join('pages', 'index.js') | ||
|
||
await ctx.actions.file.writeFileInProject( | ||
indexPath, | ||
(await ctx.file.readFileInProject(indexPath)).replace('Welcome to', 'Hello from'), | ||
) | ||
}) | ||
|
||
cy.get('.failed > .num', { timeout: 10000 }).should('contain', 1) | ||
|
||
cy.withCtx(async (ctx) => { | ||
const indexTestPath = ctx.path.join('pages', 'index.cy.js') | ||
|
||
await ctx.actions.file.writeFileInProject( | ||
indexTestPath, | ||
(await ctx.file.readFileInProject(indexTestPath)).replace('Welcome to', 'Hello from'), | ||
) | ||
}) | ||
|
||
cy.get('.passed > .num').should('contain', 1) | ||
}) | ||
|
||
it('should detect new spec', () => { | ||
cy.visitApp() | ||
|
||
cy.withCtx(async (ctx) => { | ||
const newTestPath = ctx.path.join('pages', 'New.cy.js') | ||
const indexTestPath = ctx.path.join('pages', 'index.cy.js') | ||
|
||
await ctx.actions.file.writeFileInProject( | ||
newTestPath, | ||
await ctx.file.readFileInProject(indexTestPath), | ||
) | ||
}) | ||
|
||
cy.contains('New.cy.js').click() | ||
cy.get('.passed > .num').should('contain', 1) | ||
}) | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
import type { CreateFinalWebpackConfig } from '../createWebpackDevServer' | ||
import debugLib from 'debug' | ||
import type { Configuration } from 'webpack' | ||
import * as fs from 'fs' | ||
import * as path from 'path' | ||
|
||
type PresetHandler = Omit<CreateFinalWebpackConfig, 'frameworkConfig'> | ||
|
||
const debug = debugLib('cypress:webpack-dev-server-fresh:nextHandler') | ||
|
||
export async function nextHandler ({ devServerConfig, sourceWebpackModulesResult }: PresetHandler) { | ||
const webpackConfig = await loadWebpackConfig({ devServerConfig, sourceWebpackModulesResult }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added some more comments throughout |
||
|
||
debug('resolved next.js webpack config %o', webpackConfig) | ||
|
||
checkSWC(webpackConfig, devServerConfig.cypressConfig) | ||
|
||
if (webpackConfig.watchOptions && Array.isArray(webpackConfig.watchOptions.ignored)) { | ||
webpackConfig.watchOptions = { | ||
...webpackConfig.watchOptions, | ||
ignored: [...webpackConfig.watchOptions.ignored.filter((pattern: string) => !/node_modules/.test(pattern)), '**/node_modules/!(@cypress/webpack-dev-server/dist/browser.js)**'], | ||
} | ||
|
||
debug('found options next.js watchOptions.ignored %O', webpackConfig.watchOptions.ignored) | ||
} | ||
|
||
return webpackConfig | ||
} | ||
|
||
function getNextJsPackages ({ devServerConfig }: PresetHandler) { | ||
const resolvePaths = { paths: [devServerConfig.cypressConfig.projectRoot] } | ||
const packages = {} as { loadConfig: Function, getNextJsBaseWebpackConfig: Function } | ||
|
||
try { | ||
const loadConfigPath = require.resolve('next/dist/server/config', resolvePaths) | ||
|
||
packages.loadConfig = require(loadConfigPath).default | ||
} catch (e: any) { | ||
throw new Error(`Failed to load "next/dist/server/config" with error: ${e.message ?? e}`) | ||
} | ||
|
||
try { | ||
const getNextJsBaseWebpackConfigPath = require.resolve('next/dist/build/webpack-config', resolvePaths) | ||
|
||
packages.getNextJsBaseWebpackConfig = require(getNextJsBaseWebpackConfigPath).default | ||
} catch (e: any) { | ||
throw new Error(`Failed to load "next/dist/build/webpack-config" with error: ${ e.message ?? e}`) | ||
} | ||
|
||
return packages | ||
} | ||
|
||
async function loadWebpackConfig ({ devServerConfig, sourceWebpackModulesResult }: PresetHandler): Promise<Configuration> { | ||
const { loadConfig, getNextJsBaseWebpackConfig } = getNextJsPackages({ devServerConfig, sourceWebpackModulesResult }) | ||
|
||
const nextConfig = await loadConfig('development', devServerConfig.cypressConfig.projectRoot) | ||
const runWebpackSpan = await getRunWebpackSpan() | ||
const webpackConfig = await getNextJsBaseWebpackConfig( | ||
devServerConfig.cypressConfig.projectRoot, | ||
{ | ||
buildId: `@cypress/react-${Math.random().toString()}`, | ||
config: nextConfig, | ||
dev: true, | ||
isServer: false, | ||
pagesDir: findPagesDir(devServerConfig.cypressConfig.projectRoot), | ||
entrypoints: {}, | ||
rewrites: { fallback: [], afterFiles: [], beforeFiles: [] }, | ||
...runWebpackSpan, | ||
}, | ||
) | ||
|
||
return webpackConfig | ||
} | ||
|
||
export function checkSWC ( | ||
webpackConfig: Configuration, | ||
cypressConfig: Cypress.PluginConfigOptions, | ||
) { | ||
const hasSWCLoader = webpackConfig.module?.rules?.some((rule) => { | ||
return typeof rule !== 'string' && rule.oneOf?.some( | ||
(oneOf) => (oneOf.use as any)?.loader === 'next-swc-loader', | ||
) | ||
}) | ||
|
||
// "resolvedNodePath" is only set when using the user's Node.js, which is required to compile Next.js with SWC optimizations | ||
// If it is not set, they have either explicitly set "nodeVersion" to "bundled" or are are using Cypress < 9.0.0 where it was set to "bundled" by default | ||
if (hasSWCLoader && cypressConfig.nodeVersion === 'bundled') { | ||
throw new Error(`Cypress cannot compile your Next.js application when "nodeVersion" is set to "bundled". Please remove this option from your Cypress configuration file.`) | ||
} | ||
|
||
return false | ||
} | ||
|
||
const existsSync = (file: string) => { | ||
try { | ||
fs.accessSync(file, fs.constants.F_OK) | ||
|
||
return true | ||
} catch (_) { | ||
return false | ||
} | ||
} | ||
|
||
/** | ||
* Next allows the `pages` directory to be located at either | ||
* `${projectRoot}/pages` or `${projectRoot}/src/pages`. | ||
* If neither is found, return projectRoot | ||
*/ | ||
export function findPagesDir (projectRoot: string) { | ||
// prioritize ./pages over ./src/pages | ||
let pagesDir = path.join(projectRoot, 'pages') | ||
|
||
if (existsSync(pagesDir)) { | ||
return pagesDir | ||
} | ||
|
||
pagesDir = path.join(projectRoot, 'src/pages') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should it be |
||
if (existsSync(pagesDir)) { | ||
return pagesDir | ||
} | ||
|
||
return projectRoot | ||
} | ||
|
||
// Starting with v11.1.1, a trace is required. | ||
// 'next/dist/telemetry/trace/trace' only exists since v10.0.9 | ||
// and our peerDeps support back to v8 so try-catch this import | ||
// Starting from 12.0 trace is now located in 'next/dist/trace/trace' | ||
export async function getRunWebpackSpan (): Promise<{ runWebpackSpan?: any }> { | ||
let trace: (name: string) => any | ||
|
||
try { | ||
try { | ||
trace = await import('next/dist/telemetry/trace/trace').then((m) => m.trace) | ||
|
||
return { runWebpackSpan: trace('cypress') } | ||
} catch (_) { | ||
// @ts-ignore | ||
trace = await import('next/dist/trace/trace').then((m) => m.trace) | ||
|
||
return { runWebpackSpan: trace('cypress') } | ||
} | ||
} catch (_) { | ||
return {} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,6 +66,10 @@ export function sourceRelativeWebpackModules (config: WebpackDevServerConfig) { | |
htmlWebpackPlugin: {}, | ||
} as SourceRelativeWebpackResult | ||
|
||
const cypressWebpackPath = require.resolve('@cypress/webpack-batteries-included-preprocessor', { | ||
paths: [__dirname], | ||
}) | ||
|
||
// First, we source the framework, ensuring it's sourced from the user's project and not the | ||
// Cypress binary. This is the path we use to relative-resolve the | ||
if (config.framework) { | ||
|
@@ -94,30 +98,67 @@ export function sourceRelativeWebpackModules (config: WebpackDevServerConfig) { | |
|
||
let webpackJsonPath: string | ||
|
||
try { | ||
webpackJsonPath = require.resolve('webpack/package.json', { | ||
paths: [searchRoot], | ||
}) | ||
} catch (e) { | ||
if ((e as {code?: string}).code !== 'MODULE_NOT_FOUND') { | ||
if (config.framework === 'next') { | ||
try { | ||
webpackJsonPath = require.resolve('next/dist/compiled/webpack/package.json', { | ||
paths: [searchRoot], | ||
}) | ||
} catch (e) { | ||
throw e | ||
} | ||
|
||
webpackJsonPath = require.resolve('webpack/package.json', { | ||
paths: [ | ||
require.resolve('@cypress/webpack-batteries-included-preprocessor', { | ||
paths: [__dirname], | ||
}), | ||
], | ||
}) | ||
} | ||
let webpack5 = true | ||
const importPath = path.dirname(webpackJsonPath) | ||
const webpackModule = require(path.join(importPath, 'webpack.js')) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, this stuff is all super ugly. It makes me want to have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or at the very least its own file / function in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm going to refactor this! I'm sidetracked getting the dev-servers to work with the built binary but going to return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
try { | ||
const nextConfig = require(path.resolve(config.cypressConfig.projectRoot, 'next.config.js')) | ||
|
||
if (nextConfig.webpack5 === false) { | ||
webpack5 = false | ||
} | ||
} catch (e) { | ||
// No next.config.js, assume webpack 5 | ||
} | ||
|
||
webpackModule.init(webpack5) | ||
|
||
const packageJson = require(webpackJsonPath) | ||
|
||
result.webpack.importPath = path.dirname(webpackJsonPath) | ||
result.webpack.packageJson = require(webpackJsonPath) | ||
result.webpack.module = require(result.webpack.importPath) | ||
result.webpack.majorVersion = getMajorVersion(result.webpack.packageJson, [4, 5]); | ||
result.webpack.importPath = importPath | ||
result.webpack.packageJson = { ...packageJson, version: webpack5 ? '5' : '4' } | ||
result.webpack.module = webpackModule.webpack | ||
result.webpack.majorVersion = getMajorVersion(result.webpack.packageJson, [4, 5]) | ||
} else { | ||
try { | ||
webpackJsonPath = require.resolve('webpack/package.json', { | ||
paths: [searchRoot], | ||
}) | ||
} catch (e) { | ||
if ((e as {code?: string}).code !== 'MODULE_NOT_FOUND') { | ||
throw e | ||
} | ||
|
||
webpackJsonPath = require.resolve('webpack/package.json', { | ||
paths: [cypressWebpackPath], | ||
}) | ||
} | ||
|
||
result.webpack.importPath = path.dirname(webpackJsonPath) | ||
result.webpack.packageJson = require(webpackJsonPath) | ||
result.webpack.module = require(result.webpack.importPath) | ||
result.webpack.majorVersion = getMajorVersion(result.webpack.packageJson, [4, 5]) | ||
} | ||
|
||
(Module as ModuleClass)._load = function (request, parent, isMain) { | ||
if (request === 'webpack' || request.startsWith('webpack/') && config.framework === 'next' && result.webpack.majorVersion === 4) { | ||
const resolvePath = require.resolve(request, { | ||
paths: [cypressWebpackPath], | ||
}) | ||
|
||
return originalModuleLoad(resolvePath, parent, isMain) | ||
} | ||
|
||
if (request === 'webpack' || request.startsWith('webpack/')) { | ||
const resolvePath = require.resolve(request, { | ||
paths: [searchRoot], | ||
|
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.
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'laThere 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.
Addressed in ac29f02