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 2 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
4 changes: 2 additions & 2 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ mainBuildFilters: &mainBuildFilters
branches:
only:
- develop
- issue-22147-nohoist
- zachw/issue-22525

# usually we don't build Mac app - it takes a long time
# but sometimes we want to really confirm we are doing the right thing
Expand Down Expand Up @@ -128,7 +128,7 @@ commands:
- run:
name: Check current branch to persist artifacts
command: |
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "issue-22147-nohoist" ]]; then
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "zachw/issue-22525" ]]; then
echo "Not uploading artifacts or posting install comment for this branch."
circleci-agent step halt
fi
Expand Down
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
50 changes: 39 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,8 @@ 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)

return { frameworkConfig: webpackConfig, sourceWebpackModulesResult: sourceNextWebpackDeps(devServerConfig) }
}
Expand Down Expand Up @@ -255,3 +246,40 @@ 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)
}
}

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
}
}
}
}
}
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
29 changes: 24 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,44 @@
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)
}

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

it('sources from a next-11 project', async () => {
Expand All @@ -54,6 +72,7 @@ describe('nextHandler', function () {
expectWatchOverrides(webpackConfig)
expectPagesDir(webpackConfig, projectRoot)
expectWebpackSpan(webpackConfig)
expectGlobalStyleOverrides(webpackConfig)
})

it('throws if nodeVersion is set to bundled', async () => {
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 @@ -18,7 +18,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
})
})
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 @@ -18,7 +18,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'))
})

it('sources from a @vue/[email protected] project with Vue 2', async () => {
Expand All @@ -31,6 +31,6 @@ 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'))
})
})
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'