Skip to content

Commit

Permalink
Fix minifying inline CSS comments (#19167)
Browse files Browse the repository at this point in the history
We accidentally regressed back in 9.5 and dropped support for inline CSS comments. PostCSS always parses these as pass-through (and not a syntax error), which can cause problems when minifying.

Browsers do a similar thing and ignore the comments.

To ensure we generate valid CSS, this adds support for stripping the CSS comments from the build.

--- 

Fixes #15589
Closes #17130
  • Loading branch information
Timer committed Nov 14, 2020
1 parent a32b1f4 commit 61c3db7
Show file tree
Hide file tree
Showing 14 changed files with 123 additions and 12 deletions.
8 changes: 7 additions & 1 deletion packages/next/build/webpack/plugins/css-minimizer-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import cssnanoSimple from 'cssnano-simple'
import postcss from 'postcss'
import postcssScss from 'next/dist/compiled/postcss-scss'
import postcss, { Parser } from 'postcss'
import webpack from 'webpack'
import sources from 'webpack-sources'

Expand Down Expand Up @@ -31,6 +32,11 @@ export class CssMinimizerPlugin {
...this.options.postcssOptions,
to: file,
from: file,

// We don't actually add this parser to support Sass. It can also be used
// for inline comment support. See the README:
// https://github.com/postcss/postcss-scss/blob/master/README.md#2-inline-comments-for-postcss
parser: (postcssScss as any) as Parser,
}

let input: string
Expand Down
2 changes: 1 addition & 1 deletion packages/next/compiled/postcss-loader/cjs.js

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions packages/next/compiled/postcss-scss/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
The MIT License (MIT)

Copyright 2013 Andrey Sitnik <andrey@sitnik.ru>

Permission is hereby granted, free of charge, to any person obtaining a copy of
this software and associated documentation files (the "Software"), to deal in
the Software without restriction, including without limitation the rights to
use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
the Software, and to permit persons to whom the Software is furnished to do so,
subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
1 change: 1 addition & 0 deletions packages/next/compiled/postcss-scss/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"name":"postcss-scss","main":"scss-syntax.js","author":"Andrey Sitnik <andrey@sitnik.ru>","license":"MIT"}
1 change: 1 addition & 0 deletions packages/next/compiled/postcss-scss/scss-syntax.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion packages/next/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
"node-html-parser": "1.4.9",
"path-browserify": "1.0.1",
"pnp-webpack-plugin": "1.6.4",
"postcss": "8.1.1",
"postcss": "8.1.7",
"process": "0.11.10",
"prop-types": "15.7.2",
"raw-body": "2.4.1",
Expand Down Expand Up @@ -204,6 +204,7 @@
"postcss-flexbugs-fixes": "4.2.1",
"postcss-loader": "4.0.3",
"postcss-preset-env": "6.7.0",
"postcss-scss": "3.0.4",
"recast": "0.18.5",
"resolve": "1.11.0",
"semver": "7.3.2",
Expand Down
16 changes: 16 additions & 0 deletions packages/next/taskfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,21 @@ export async function ncc_postcss_preset_env(task, opts) {
.target('compiled/postcss-preset-env')
}
// eslint-disable-next-line camelcase
externals['postcss-scss'] = 'next/dist/compiled/postcss-scss'
export async function ncc_postcss_scss(task, opts) {
await task
.source(opts.src || relative(__dirname, require.resolve('postcss-scss')))
.ncc({
packageName: 'postcss-scss',
externals: {
postcss: 'postcss',
'postcss/lib/parser': 'postcss/lib/parser',
...externals,
},
})
.target('compiled/postcss-scss')
}
// eslint-disable-next-line camelcase
externals['recast'] = 'next/dist/compiled/recast'
export async function ncc_recast(task, opts) {
await task
Expand Down Expand Up @@ -584,6 +599,7 @@ export async function ncc(task) {
'ncc_postcss_flexbugs_fixes',
'ncc_postcss_loader',
'ncc_postcss_preset_env',
'ncc_postcss_scss',
'ncc_recast',
'ncc_resolve',
'ncc_schema_utils',
Expand Down
4 changes: 4 additions & 0 deletions packages/next/types/misc.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ declare module 'next/dist/compiled/semver' {
import m from 'semver'
export = m
}
declare module 'next/dist/compiled/postcss-scss' {
import m from 'postcss-scss'
export = m
}
declare module 'next/dist/compiled/text-table' {
function textTable(
rows: Array<Array<{}>>,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"browserslist": [
"last 1 chrome version"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import './global.css'

export default function MyApp({ Component, pageProps }) {
return <Component {...pageProps} />
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Aaaand we are dead
// These comments are invalid CSS and it crashes the build since Next.js 9.5.0

* {
box-sizing: border-box;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function () {
return <div></div>
}
32 changes: 32 additions & 0 deletions test/integration/css-features/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,35 @@ describe('CSS Modules: Import Exports', () => {
)
})
})

describe('Inline Comments: Minify', () => {
const appDir = join(fixturesDir, 'inline-comments')

let stdout
let code
beforeAll(async () => {
await remove(join(appDir, '.next'))
;({ code, stdout } = await nextBuild(appDir, [], {
stdout: true,
}))
})

it('should have compiled successfully', () => {
expect(code).toBe(0)
expect(stdout).toMatch(/Compiled successfully/)
})

it(`should've emitted a single CSS file`, async () => {
const cssFolder = join(appDir, '.next/static/css')

const files = await readdir(cssFolder)
const cssFiles = files.filter((f) => /\.css$/.test(f))

expect(cssFiles.length).toBe(1)
const cssContent = await readFile(join(cssFolder, cssFiles[0]), 'utf8')

expect(cssContent.replace(/\/\*.*?\*\//g, '').trim()).toMatchInlineSnapshot(
`"*{box-sizing:border-box}"`
)
})
})
Loading

0 comments on commit 61c3db7

Please sign in to comment.