Skip to content

Commit

Permalink
Fix CSS modules imports from outside of the root directory (#42106)
Browse files Browse the repository at this point in the history
Fixes #41856. There is no way to apply those loader rules to specific
directories, especially when `transpileModules` is used. This PR changes
that to be based on the issuer layer.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
shuding and ijjk committed Oct 31, 2022
1 parent 005cf9b commit 2280a2d
Show file tree
Hide file tree
Showing 14 changed files with 51 additions and 40 deletions.
37 changes: 4 additions & 33 deletions packages/next/build/webpack/config/blocks/css/index.ts
Expand Up @@ -273,6 +273,7 @@ export const css = curry(async function css(
// CSS Modules support must be enabled on the server and client so the class
// names are available for SSR or Prerendering.
if (ctx.hasAppDir && !ctx.isProduction) {
// CSS modules
fns.push(
loader({
oneOf: [
Expand All @@ -284,16 +285,6 @@ export const css = curry(async function css(
sideEffects: false,
// CSS Modules are activated via this specific extension.
test: regexCssModules,
// CSS Modules are only supported in the user's application. We're
// not yet allowing CSS imports _within_ `node_modules`.
issuer: {
and: [
{
or: [ctx.rootDirectory, regexClientEntry],
},
],
not: [/node_modules/],
},
use: [
require.resolve('../../../loaders/next-flight-css-dev-loader'),
...getCssModuleLoader(ctx, lazyPostCSSInitializer),
Expand All @@ -302,10 +293,12 @@ export const css = curry(async function css(
],
})
)

// Opt-in support for Sass (using .scss or .sass extensions).
fns.push(
loader({
oneOf: [
// Opt-in support for Sass (using .scss or .sass extensions).
// For app dir, we match both server and client layers.
markRemovable({
// Sass Modules should never have side effects. This setting will
// allow unused Sass to be removed from the production build.
Expand All @@ -314,12 +307,6 @@ export const css = curry(async function css(
sideEffects: false,
// Sass Modules are activated via this specific extension.
test: regexSassModules,
// Sass Modules are only supported in the user's application. We're
// not yet allowing Sass imports _within_ `node_modules`.
issuer: {
and: [ctx.rootDirectory],
not: [/node_modules/],
},
use: [
require.resolve('../../../loaders/next-flight-css-dev-loader'),
...getCssModuleLoader(
Expand All @@ -344,16 +331,6 @@ export const css = curry(async function css(
sideEffects: false,
// CSS Modules are activated via this specific extension.
test: regexCssModules,
// CSS Modules are only supported in the user's application. We're
// not yet allowing CSS imports _within_ `node_modules`.
issuer: {
and: [
{
or: [ctx.rootDirectory, regexClientEntry],
},
],
not: [/node_modules/],
},
use: getCssModuleLoader(ctx, lazyPostCSSInitializer),
}),
],
Expand All @@ -371,12 +348,6 @@ export const css = curry(async function css(
sideEffects: false,
// Sass Modules are activated via this specific extension.
test: regexSassModules,
// Sass Modules are only supported in the user's application. We're
// not yet allowing Sass imports _within_ `node_modules`.
issuer: {
and: [ctx.rootDirectory],
not: [/node_modules/],
},
use: getCssModuleLoader(
ctx,
lazyPostCSSInitializer,
Expand Down
1 change: 1 addition & 0 deletions test/e2e/app-dir/app-alias/next.config.js
@@ -1,5 +1,6 @@
module.exports = {
experimental: {
appDir: true,
transpileModules: ['ui'],
},
}
20 changes: 20 additions & 0 deletions test/e2e/app-dir/rsc-external.test.ts
Expand Up @@ -135,4 +135,24 @@ describe('app dir - rsc external dependency', () => {
expect(result).toMatch(/\.css/)
})
})

it('should handle external css modules', async () => {
const browser = await webdriver(next.url, '/css/modules')

expect(
await browser.eval(
`window.getComputedStyle(document.querySelector('h1')).color`
)
).toBe('rgb(255, 0, 0)')
})

it('should handle external css modules in pages', async () => {
const browser = await webdriver(next.url, '/test-pages')

expect(
await browser.eval(
`window.getComputedStyle(document.querySelector('h1')).color`
)
).toBe('rgb(255, 0, 0)')
})
})
2 changes: 1 addition & 1 deletion test/e2e/app-dir/rsc-external/app/css/[...slug]/page.js
@@ -1,4 +1,4 @@
import 'global-css/style.css'
import 'css/style.css'

export default function Page() {
return <div>hello</div>
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/rsc-external/app/css/modules/page.js
@@ -0,0 +1,5 @@
import Foo from 'css/module'

export default function Page() {
return <Foo />
}
2 changes: 1 addition & 1 deletion test/e2e/app-dir/rsc-external/next.config.js
Expand Up @@ -3,6 +3,6 @@ module.exports = {
experimental: {
appDir: true,
serverComponentsExternalPackages: ['conditional-exports-optout'],
transpilePackages: ['untranspiled-module'],
transpilePackages: ['untranspiled-module', 'css'],
},
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/rsc-external/node_modules_bak/css/module.js
@@ -0,0 +1,5 @@
import styles from './styles.module.css'

export default function Foo() {
return <h1 className={styles.h1}>Hello</h1>
}
@@ -1,10 +1,11 @@
{
"name": "global-css",
"name": "css",
"type": "module",
"sideEffects": false,
"exports": {
".": "./index.js",
"./style.css": "./style.css",
"./module": "./module.js",
"./package.json": "./package.json"
}
}
@@ -0,0 +1,3 @@
.h1 {
color: red;
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/rsc-external/pages/test-pages.jsx
@@ -0,0 +1,5 @@
import Foo from 'css/module'

export default function Page() {
return <Foo />
}
4 changes: 2 additions & 2 deletions test/integration/css-modules/test/index.test.js
Expand Up @@ -237,7 +237,7 @@ describe('Can hot reload CSS Module without losing state', () => {
})
})

describe('Invalid CSS Module Usage in node_modules', () => {
describe.skip('Invalid CSS Module Usage in node_modules', () => {
const appDir = join(fixturesDir, 'invalid-module')

beforeAll(async () => {
Expand All @@ -258,7 +258,7 @@ describe('Invalid CSS Module Usage in node_modules', () => {
})
})

describe('Invalid Global CSS Module Usage in node_modules', () => {
describe.skip('Invalid Global CSS Module Usage in node_modules', () => {
const appDir = join(fixturesDir, 'invalid-global-module')

beforeAll(async () => {
Expand Down
4 changes: 2 additions & 2 deletions test/integration/scss-modules/test/index.test.js
Expand Up @@ -233,7 +233,7 @@ describe('Can hot reload CSS Module without losing state', () => {
})
})

describe('Invalid CSS Module Usage in node_modules', () => {
describe.skip('Invalid CSS Module Usage in node_modules', () => {
const appDir = join(fixturesDir, 'invalid-module')

beforeAll(async () => {
Expand All @@ -254,7 +254,7 @@ describe('Invalid CSS Module Usage in node_modules', () => {
})
})

describe('Invalid CSS Global Module Usage in node_modules', () => {
describe.skip('Invalid CSS Global Module Usage in node_modules', () => {
const appDir = join(fixturesDir, 'invalid-global-module')

beforeAll(async () => {
Expand Down

0 comments on commit 2280a2d

Please sign in to comment.