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

[next] 🐞 collect() fails on a particular CSS snippet from bootstrap 4 #217

Closed
ivan-aksamentov opened this issue Sep 17, 2018 · 4 comments
Assignees
Labels
bug 🐛 Issue is a confirmed bug needs: response 📩 Response from author/commenters is requested

Comments

@ivan-aksamentov
Copy link

ivan-aksamentov commented Sep 17, 2018

Do you want to request a feature or report a bug?
🐞 bug

What is the current behavior?
While server rendering, during collection of critical CSS, the following error is being emitted:

TypeError: Cannot read property 'match' of undefined
    at main.css:39:3
    at .dev/server/webpack:/node_modules/linaria/lib/server/collect.js:30:1
    at AtRule.each (.dev/server/webpack:/node_modules/linaria/node_modules/postcss/lib/container.js:110:1)
    at handleAtRule (.dev/server/webpack:/node_modules/linaria/lib/server/collect.js:29:1)
    at .dev/server/webpack:/node_modules/linaria/lib/server/collect.js:53:1
    at .dev/server/webpack:/node_modules/linaria/node_modules/postcss/lib/container.js:247:1
    at .dev/server/webpack:/node_modules/linaria/node_modules/postcss/lib/container.js:146:1
    at AtRule.each (.dev/server/webpack:/node_modules/linaria/node_modules/postcss/lib/container.js:110:1)
    at AtRule.walk (.dev/server/webpack:/node_modules/linaria/node_modules/postcss/lib/container.js:143:1)
    at .dev/server/webpack:/node_modules/linaria/node_modules/postcss/lib/container.js:156:1
    at Root.each (.dev/server/webpack:/node_modules/linaria/node_modules/postcss/lib/container.js:110:1)

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

In my crazy setup, linaria receives a particular snippet of CSS, originally from Bootstrap 4 SASS, via sass-loader -> postcss-loader -> css-loader -> extract-css-chunks-webpack-plugin -> webpack-flush-chunks, via some hackery. So far so good, but when I proceeded to server-renderer, collect() choked on it.
Here is this snippet:

@media print {
  h2,
  h3 {
    page-break-after: avoid;
  }

  @page {
    size: a3;
  }

  body {
    min-width: 992px !important;
  }
}

It is reproducible without bootstrap/sass/postcss. Just paste the snippet and linaria will gladly kill the server.
Here is the repro:

  • First build and run:
git clone https://github.com/ivan-aksamentov/reactlandia-bolerplate-lite -b  repro/linaria-collect 
cd reactlandia-bolerplate-lite
git checkout c86b1938ad7d58f4d693ba848144cb12358b3ec6
npm install
npm run dev
  • Then curl or open in browser http://localhost:3000
  • Boom! 💥

Files of interest:

What is the expected behavior?

  • collect() should return successfully
  • critical CSS should be extracted correctly

Please provide your exact Babel configuration and mention your Linaria, Node, Yarn/npm version and operating system.

babel.config.js:

module.exports = {
  'presets': [
    '@babel/preset-env',
    '@babel/preset-react',
    ['linaria/babel', { 'evaluate': true }],
  ],
  'plugins': [
    '@babel/plugin-transform-runtime',
    '@babel/plugin-proposal-class-properties',
    '@babel/plugin-proposal-object-rest-spread',
    '@babel/plugin-syntax-dynamic-import',
    'universal-import',
  ],
}

versions:

Ubunty 16.04
node 10.10.0
npm 6.4.1
babel 7.0.0
linaria/next/1e17772f3411ceed6c013fb7e9cf85edcf8f4390

Workaround

Well, obviously, I could (and I did) shut up the error by adding null-checks in conditional in /src/server/collect.js:27:

     let addedToCritical = false;
 
     rule.each(childRule => {
-      if (childRule.selector.match(htmlClassesRegExp)) {
+      if (
+        childRule &&
+        childRule.selector &&
+        childRule.selector.match(htmlClassesRegExp)
+      ) {
         critical.append(rule.clone());
         addedToCritical = true;
       }

However, I don't fully understand (yet?) how collect() works, and whether this workaround will affect the correctness. So, I think this should be considered by maintainers.
CC: @thymikee @satya164

@satya164 satya164 added the bug 🐛 Issue is a confirmed bug label Sep 18, 2018
@satya164
Copy link
Member

I can successfully repro this. The following is enough to repro:

@media print {
  h2,
  h3 {
    page-break-after: avoid;
  }

  @page {
    size: a3;
  }
}

Strangely either of the following don't exhibit the issue:

@media print {
  h2,
  h3 {
    page-break-after: avoid;
  }
}

Or

@media print {
  @page {
    size: a3;
  }
}

Any idea why it would happen @thymikee? I tried to fix it, but I'm not familiar with postcss yet.

@thymikee
Copy link
Member

Not sure, I don't have a way to check it currently

@jayu
Copy link
Contributor

jayu commented Mar 26, 2020

Does it get fixed? It's been a while since the issue was created.

@jayu jayu added the needs: response 📩 Response from author/commenters is requested label Mar 26, 2020
@jayu
Copy link
Contributor

jayu commented Apr 3, 2020

I assume it's outdated

@jayu jayu closed this as completed Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Issue is a confirmed bug needs: response 📩 Response from author/commenters is requested
Projects
None yet
Development

No branches or pull requests

4 participants