Skip to content
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

fix: allow next.js apps to import global styles #22769

Merged
merged 17 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions npm/webpack-dev-server/cypress/e2e/next.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,12 @@ for (const project of WEBPACK_REACT) {
cy.waitForSpecToFinish()
cy.get('.passed > .num').should('contain', 1)
})

it('should allow import of global styles in support file', () => {
cy.visitApp()
cy.contains('styles.cy.js').click()
cy.waitForSpecToFinish()
cy.get('.passed > .num').should('contain', 1)
})
})
}
6 changes: 4 additions & 2 deletions npm/webpack-dev-server/src/devServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,11 @@ export function devServer (devServerConfig: WebpackDevServerConfig): Promise<Cyp
})
}

export type PresetHandlerResult = { frameworkConfig?: Configuration, sourceWebpackModulesResult: SourceRelativeWebpackResult }
export type PresetHandlerResult = { frameworkConfig: Configuration, sourceWebpackModulesResult: SourceRelativeWebpackResult }

async function getPreset (devServerConfig: WebpackDevServerConfig): Promise<PresetHandlerResult> {
type Optional<T, K extends keyof T> = Pick<Partial<T>, K> & Omit<T, K>

async function getPreset (devServerConfig: WebpackDevServerConfig): Promise<Optional<PresetHandlerResult, 'frameworkConfig'>> {
switch (devServerConfig.framework) {
case 'create-react-app':
return createReactAppHandler(devServerConfig)
Expand Down
67 changes: 56 additions & 11 deletions npm/webpack-dev-server/src/helpers/nextHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,9 @@ export async function nextHandler (devServerConfig: WebpackDevServerConfig): Pro
debug('resolved next.js webpack config %o', webpackConfig)

checkSWC(webpackConfig, devServerConfig.cypressConfig)

// Next webpack compiler ignored watching any node_modules changes, but we need to watch
// for changes to 'dist/browser.js' in order to detect new specs that have been added
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)
}
watchEntryPoint(webpackConfig)
allowGlobalStylesImports(webpackConfig)
changeNextCachePath(webpackConfig)

return { frameworkConfig: webpackConfig, sourceWebpackModulesResult: sourceNextWebpackDeps(devServerConfig) }
}
Expand Down Expand Up @@ -255,3 +247,56 @@ function sourceNextWebpack (devServerConfig: WebpackDevServerConfig, framework:

return webpack
}

// Next webpack compiler ignored watching any node_modules changes, but we need to watch
// for changes to 'dist/browser.js' in order to detect new specs that have been added
function watchEntryPoint (webpackConfig: Configuration) {
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)
}
}

// We are matching the Next.js regex rules exactly. If we were writing our own loader, we could
// condense these regex rules into a single rule but we need the regex.source to be identical to what
// we get from Next.js webpack config
// see: https://github.com/vercel/next.js/blob/20486c159d8538a337da6b07b0b4490a3a0d6b91/packages/next/build/webpack/config/blocks/css/index.ts#L18
const globalCssRe = [/(?<!\.module)\.css$/, /(?<!\.module)\.(scss|sass)$/]
const globalCssModulesRe = [/\.module\.css$/, /\.module\.(scss|sass)$/]
ZachJW34 marked this conversation as resolved.
Show resolved Hide resolved

export const allCssTests = [...globalCssRe, ...globalCssModulesRe]

// Next does not allow global styles to be loaded outside of the main <App /> component.
// We want users to be able to import the global styles into their component support file so we
// delete the "issuer" from the rules that process css/scss files.
// see: https://github.com/cypress-io/cypress/issues/22525
// Motivated by: https://github.com/bem/next-global-css
function allowGlobalStylesImports (webpackConfig: Configuration) {
const rules = webpackConfig.module?.rules || []

for (const rule of rules) {
if (typeof rule !== 'string' && rule.oneOf) {
for (const oneOf of rule.oneOf) {
if (oneOf.test && allCssTests.some((re) => re.source === (oneOf as any).test?.source)) {
delete oneOf.issuer
}
}
}
}
}

// Our modifications of the Next webpack config can corrupt the cache used for local development.
// We separate the cache used for CT from the normal cache (".next/cache/webpack" -> ".next/cache/cypress-webpack") so they don't interfere with each other
function changeNextCachePath (webpackConfig: Configuration) {
if (typeof webpackConfig.cache === 'object' && ('cacheDirectory' in webpackConfig.cache) && webpackConfig.cache.cacheDirectory) {
const { cacheDirectory } = webpackConfig.cache

webpackConfig.cache.cacheDirectory = cacheDirectory.replace(/webpack$/, 'cypress-webpack')

debug('Changing Next cache path from %s to %s', cacheDirectory, webpackConfig.cache.cacheDirectory)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const expectEslintModifications = (webpackConfig: Configuration) => {
}

const expectModuleSourceInPlaceModifications = (webpackConfig: Configuration, projectRoot: string) => {
const moduleSourcePlugin: any = webpackConfig.resolve.plugins.find((plugin) => plugin.constructor.name === 'ModuleScopePlugin')
const moduleSourcePlugin: any = webpackConfig.resolve?.plugins?.find((plugin) => plugin.constructor.name === 'ModuleScopePlugin')

if (!moduleSourcePlugin) {
throw new Error('Expected to find ModuleScopePlugin in webpack config')
Expand All @@ -27,7 +27,7 @@ const expectModuleSourceInPlaceModifications = (webpackConfig: Configuration, pr
}

const expectBabelRuleModifications = (webpackConfig: Configuration, projectRoot: string) => {
const babelRule: any = (webpackConfig.module.rules as any).find((rule) => rule.oneOf)?.oneOf.find((oneOf) => oneOf.loader?.includes('babel-loader'))
const babelRule: any = (webpackConfig.module?.rules as any)?.find((rule) => rule.oneOf)?.oneOf.find((oneOf) => oneOf.loader?.includes('babel-loader'))

if (!babelRule) {
throw new Error('Expected to find BabelRule in webpack config')
Expand All @@ -37,7 +37,7 @@ const expectBabelRuleModifications = (webpackConfig: Configuration, projectRoot:
}

const expectReactScriptsFiveModifications = (webpackConfig: Configuration) => {
const definePlugin: any = webpackConfig.plugins.find((plugin) => plugin.constructor.name === 'DefinePlugin')
const definePlugin: any = webpackConfig.plugins?.find((plugin) => plugin.constructor.name === 'DefinePlugin')

if (!definePlugin) {
throw new Error('Expected to find DefinePlugin in webpack config')
Expand Down
44 changes: 39 additions & 5 deletions npm/webpack-dev-server/test/handlers/nextHandler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,55 @@
import { scaffoldMigrationProject } from '../test-helpers/scaffoldProject'
import { expect } from 'chai'
import { nextHandler } from '../../src/helpers/nextHandler'
import type { Configuration } from 'webpack'
import { nextHandler, allCssTests } from '../../src/helpers/nextHandler'
import type { Configuration, RuleSetRule } from 'webpack'
import * as path from 'path'
import { WebpackDevServerConfig } from '../../src/devServer'
import '../support'

const expectWatchOverrides = (webpackConfig: Configuration) => {
expect(webpackConfig.watchOptions.ignored).to.contain('**/node_modules/!(@cypress/webpack-dev-server/dist/browser.js)**')
expect(webpackConfig.watchOptions?.ignored).to.contain('**/node_modules/!(@cypress/webpack-dev-server/dist/browser.js)**')
}

const expectPagesDir = (webpackConfig: Configuration, projectRoot: string) => {
const ReactLoadablePlugin: any = webpackConfig.plugins.find((plugin) => plugin.constructor.name === 'ReactLoadablePlugin')
const ReactLoadablePlugin: any = webpackConfig.plugins?.find((plugin) => plugin.constructor.name === 'ReactLoadablePlugin')

expect(ReactLoadablePlugin.pagesDir).eq(path.join(projectRoot, 'pages'))
}

const expectWebpackSpan = (webpackConfig: Configuration) => {
const ProfilingPlugin: any = webpackConfig.plugins.find((plugin) => plugin.constructor.name === 'ProfilingPlugin')
const ProfilingPlugin: any = webpackConfig.plugins?.find((plugin) => plugin.constructor.name === 'ProfilingPlugin')

expect(ProfilingPlugin.runWebpackSpan).to.exist
}

const expectGlobalStyleOverrides = (webpackConfig: Configuration) => {
const cssRules: RuleSetRule[] = []

for (const rule of webpackConfig.module?.rules as RuleSetRule[]) {
if (rule.oneOf) {
for (const oneOf of rule.oneOf) {
if (oneOf.test && allCssTests.some((re) => re.source === (oneOf as any).test?.source)) {
cssRules.push(oneOf)
}
}
}
}

expect(cssRules).to.have.length.greaterThan(0)
cssRules.forEach((rule) => expect(rule.issuer).to.be.undefined)
}

const expectCacheOverrides = (webpackConfig: Configuration, projectRoot: string) => {
const cache: any = webpackConfig.cache

// No cache for Webpack 4
if (!cache || !cache.cacheDirectory) {
return
}

expect(cache.cacheDirectory).eq(path.join(projectRoot, '.next', 'cache', 'cypress-webpack'))
}

describe('nextHandler', function () {
// can take a while since we install node_modules
this.timeout(1000 * 60)
Expand All @@ -39,6 +67,8 @@ describe('nextHandler', function () {
expectWatchOverrides(webpackConfig)
expectPagesDir(webpackConfig, projectRoot)
expectWebpackSpan(webpackConfig)
expectGlobalStyleOverrides(webpackConfig)
expectCacheOverrides(webpackConfig, projectRoot)

expect(sourceWebpackModulesResult.webpack.importPath).to.include('next')
expect(sourceWebpackModulesResult.webpack.majorVersion).eq(5)
Expand All @@ -57,6 +87,8 @@ describe('nextHandler', function () {
expectWatchOverrides(webpackConfig)
expectPagesDir(webpackConfig, projectRoot)
expectWebpackSpan(webpackConfig)
expectGlobalStyleOverrides(webpackConfig)
expectCacheOverrides(webpackConfig, projectRoot)

expect(sourceWebpackModulesResult.webpack.importPath).to.include('next')
expect(sourceWebpackModulesResult.webpack.majorVersion).eq(5)
Expand All @@ -75,6 +107,8 @@ describe('nextHandler', function () {
expectWatchOverrides(webpackConfig)
expectPagesDir(webpackConfig, projectRoot)
expectWebpackSpan(webpackConfig)
expectGlobalStyleOverrides(webpackConfig)
expectCacheOverrides(webpackConfig, projectRoot)

expect(sourceWebpackModulesResult.webpack.importPath).to.include('next')
expect(sourceWebpackModulesResult.webpack.majorVersion).eq(4)
Expand Down
2 changes: 1 addition & 1 deletion npm/webpack-dev-server/test/handlers/nuxtHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('nuxtHandler', function () {
} as WebpackDevServerConfig)

// Verify it's a Vue-specific webpack config by seeing if VueLoader is present.
expect(webpackConfig.plugins.find((plug) => plug.constructor.name === 'VueLoader'))
expect(webpackConfig.plugins?.find((plug) => plug.constructor.name === 'VueLoader'))
expect(webpackConfig.performance).to.be.undefined

expect(sourceWebpackModulesResult.framework?.importPath).to.include('@nuxt/webpack')
Expand Down
4 changes: 2 additions & 2 deletions npm/webpack-dev-server/test/handlers/vueCliHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('vueCliHandler', function () {
} as WebpackDevServerConfig)

// Verify it's a Vue-specific webpack config by seeing if VueLoader is present.
expect(webpackConfig.plugins.find((plug) => plug.constructor.name === 'VueLoader'))
expect(webpackConfig.plugins?.find((plug) => plug.constructor.name === 'VueLoader'))

expect(sourceWebpackModulesResult.framework?.importPath).to.include('@vue/cli-service')
expect(sourceWebpackModulesResult.webpack.majorVersion).eq(5)
Expand All @@ -36,7 +36,7 @@ describe('vueCliHandler', function () {
} as WebpackDevServerConfig)

// Verify it's a Vue-specific webpack config by seeing if VueLoader is present.
expect(webpackConfig.plugins.find((plug) => plug.constructor.name === 'VueLoader'))
expect(webpackConfig.plugins?.find((plug) => plug.constructor.name === 'VueLoader'))

expect(sourceWebpackModulesResult.framework?.importPath).to.include('@vue/cli-service')
expect(sourceWebpackModulesResult.webpack.majorVersion).eq(4)
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
//
// Importing global styles fails with Next.js due to restrictions on style imports.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to add some kind of assertion that the global style is actually applied to some element?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an assertion here: b0e8bf0

// We modify the Next Webpack config to allow importing global styles.
import '../../styles/globals.css'
import '../../styles/Home.module.css'
12 changes: 12 additions & 0 deletions system-tests/project-fixtures/next/pages/styles.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { mount } from "cypress/react"
import Index from "./index.js"

describe('<Index />', () => {
it('renders', () => {
mount(<Index />)
cy.contains('h1', 'Welcome to Next.js!')
// Verifying that global styles can be imported into support file:
// Relevant file: system-tests/project-fixtures/next/styles/globals.css
cy.get('body').should('have.css', 'background-color', 'rgb(204, 255, 255)')
})
})
1 change: 1 addition & 0 deletions system-tests/project-fixtures/next/styles/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ body {
margin: 0;
font-family: -apple-system, BlinkMacSystemFont, Segoe UI, Roboto, Oxygen,
Ubuntu, Cantarell, Fira Sans, Droid Sans, Helvetica Neue, sans-serif;
background-color: rgb(204, 255, 255);
}

a {
Expand Down