search

Siilwyn / css-declaration-sorter

Sort CSS declarations fast and automatically in a certain order.
https://npmjs.com/css-declaration-sorter
ISC License
331 stars 16 forks source link

TypeError: Cannot read property 'insertAfter' of undefined #170

Closed dave105010 closed 3 years ago

dave105010 commented 3 years ago

Hi, I keep getting this error. I did "yarn cache clean" Reinstalled everything and still cannot figure out why this is failing.

Done in 38.91s.
Compiling...
Compilation failed:
warning package.json: No license field
TypeError: Cannot read property 'insertAfter' of undefined
    at node_modules/optimize-css-assets-webpack-plugin/node_modules/css-declaration-sorter/src/main.cjs:372:22
    at Array.forEach (<anonymous>)
    at processCss (node_modules/optimize-css-assets-webpack-plugin/node_modules/css-declaration-sorter/src/main.cjs:369:12)
    at node_modules/optimize-css-assets-webpack-plugin/node_modules/css-declaration-sorter/src/main.cjs:304:21
    at async LazyResult.runAsync (node_modules/optimize-css-assets-webpack-plugin/node_modules/postcss/lib/lazy-result.js:431:15)
    at async Promise.all (index 0)
Siilwyn commented 3 years ago

Well there is no insertAfter used in the code of this project. Could you share a minimal reproducible example?

philJohnson commented 3 years ago

I'm having the same issues when running a gatsby build of my project - https://github.com/philJohnson/thelippyj

I'm trying to look into why today so I may have an idea after.

philJohnson commented 3 years ago

After quite a bit of digging I found the error I was getting was due to RTLCSS control directives in an included SCSS file. Once I removed these the build succeeded without issue.

/*!rtl:begin:ignore*/
grid-column: 1;
/*!rtl:end:ignore*/
yunwuxin commented 3 years ago

I'm having the same issues

Siilwyn commented 3 years ago

@philJohnson thanks for the info but I can't figure it out, could you give a minimal reproducible code example?

I've started off with this, which I expect to error out:

import postcss from 'postcss';
import cssDeclarationSorter from 'css-declaration-sorter';

postcss([cssDeclarationSorter()])
  .process(`a {
/*!rtl:begin:ignore*/
grid-column: 1;
/*!rtl:end:ignore*/
}`, { from: undefined })
  .then(result =>
    console.log(result.css)
  );
Siilwyn commented 3 years ago

This issue needs an example to reproduce, closing for now, please provide one and/or shoot a PR with a test that fails.

kraftner commented 3 years ago

Just hit this as well.

@Siilwyn it seems that there needs to be something after as well for it to fail.

This should trigger the error:

.class {
  /*!rtl:begin:ignore*/
  direction: ltr;
  /*!rtl:end:ignore*/
  display: grid;
}
Siilwyn commented 3 years ago

Hey thanks for the extra info @kraftner however I still can't reproduce, with:

import postcss from 'postcss';
import cssDeclarationSorter from 'css-declaration-sorter';

postcss([cssDeclarationSorter()])
  .process(`.class {
  /*!rtl:begin:ignore*/
  direction: ltr;
  /*!rtl:end:ignore*/
  display: grid;
}`, { from: undefined })
  .then(result =>
    console.log(result.css)
  );

Could you provide something I can reproduce? That way I can add a test (to prevent regressions) and find the actual issue.

kraftner commented 3 years ago

First of all: Sorry for me being lazy and not offering an actual reduced test-case.

I was trying to do this now. I am actually using https://github.com/JeffreyWay/laravel-mix and was (yet) only able to reproduce it there. Although the error seems to originate from your package I am now not sure anymore if it really is the source of the problem here.

So, I've created a gist with the testcase reduced as much as I managed for now. Just run npm run build to see the error:

ERROR in ./test.scss Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js): ModuleBuildError: Module build failed (from ./node_modules/postcss-loader/dist/cjs.js): TypeError: Cannot read property 'insertAfter' of undefined at /usr/var/app/node_modules/css-declaration-sorter/src/main.cjs:382:22 at Array.forEach () at processCss (/usr/var/app/node_modules/css-declaration-sorter/src/main.cjs:379:12) at /usr/var/app/node_modules/css-declaration-sorter/src/main.cjs:314:33 at async LazyResult.runAsync (/usr/var/app/node_modules/postcss/lib/lazy-result.js:431:15) at async Object.loader (/usr/var/app/node_modules/postcss-loader/dist/index.js:97:14) at processResult (/usr/var/app/node_modules/webpack/lib/NormalModule.js:713:19) at /usr/var/app/node_modules/webpack/lib/NormalModule.js:819:5 at /usr/var/app/node_modules/loader-runner/lib/LoaderRunner.js:399:11 at /usr/var/app/node_modules/loader-runner/lib/LoaderRunner.js:251:18 at context.callback (/usr/var/app/node_modules/loader-runner/lib/LoaderRunner.js:124:13) at Object.loader (/usr/var/app/node_modules/postcss-loader/dist/index.js:142:7)

As soon as you uncomment outputStyle: 'expanded' in webpack.mix.js the error goes away. It also goes away if you don't run in production mode (so mix instead of mix --production.)

I will try to reduce this further but since I am not very familiar with all of this I thought I'd share my intermediate findings since you might already be able to see something useful in there. Even if it's just that I should better file this big with laravel-mix, sass-loader or anything else.

Siilwyn commented 3 years ago

Thanks for the reproducible example @kraftner!

The error is thrown from https://github.com/Siilwyn/css-declaration-sorter/blob/master/src/main.mjs#L100, using a different outputStyle seems like a decent workaround for now?

I've logged the node and pairedNode, both are missing a parent:

  pairedNode: Comment {
    raws: { before: '', left: '', right: '' },
    type: 'comment',
    parent: undefined,
    source: { start: [Object], input: [Input], end: [Object] },
    text: '!rtl:end:ignore',
    [Symbol(isClean)]: true,
    [Symbol(my)]: true
  }

Which as far as I understand shouldn't be possible, the comment !rtl:end:ignore does not belong to a parent/selector, it should belong to .class. :thinking:

Is it expected that display: grid; is removed from the output? Seems to be removed if outputStyle is unset. In other words, (should have started with this question from the beginning), what is the expected output?

kraftner commented 3 years ago

Yes, setting the outputStyle works a as a workaround. :+1:

Concerning the missing display: grid; I was sloppy with my demo, in contrast to what I posted in the issue it also doesn't have it in the source file and if you add it to the source it also gets compiled properly.

The only thing I'm really expecting is no error thrown. :smile:

Thinking about it further I was wondering if the reason things are tripping might have something to do that node-sass defaults to nested for outputStyle while dart-sass doesn't support that at all. So maybe it's just an unhandled edge case. But that is only wild guessing to be honest.

Still you might want to failsafe on the missing parent? To me it looks like the other people in this issue hit that error via other paths.

Siilwyn commented 3 years ago

@kraftner nice find, that could very well be the issue here!

The question is if I failsafe what should happen with the detached comment, if this module suddenly omits comments from the output that would be even worse I think.

kraftner commented 3 years ago

Yeah, that makes sense too... :thinking:

Maybe it is worth checking in with postcss to see how the seemingly "this should never happen" situation can actually happen. But as I've said earlier unfortunately I know really nothing about postcss internals so probably not of much help here...

zfeher commented 3 years ago

We also encountered this error. Currently we are migrating over webpack 5 from webpack 4 and we also update all build related dependencies (like cssnano which in the end uses css-declaration-sorter). Reading through these comments we managed to make it work via explicitly setting outputStyle of node-sass to the default nested value via sass-loader sassOptions. After that the production build/serve didn't throw the error.

Interestingly the develoment build/serve works without this.

YetiCGN commented 3 years ago

Well there is no insertAfter used in the code of this project. Could you share a minimal reproducible example?

No, but an 'insert' + node.insertPosition in pairedNode.parent['insert' + node.insertPosition](pairedNode, node.comment); which is then evaluated to insertAfter.

css-declaration-sorter 6.0.3 & 6.1.1, src/main.cjs line 372 (in 6.0.3) or 382 (in 6.1.1)

Siilwyn commented 3 years ago

@zfeher @dave105010 you are not using laravel-mix right? If you could provide an example using just webpack (as minimal as possible) that would help me out a ton!

Edit: Cleaned up the thread by hiding some comments which provide no new info, please read the posted replies so far before posting.

dave105010 commented 3 years ago

@zfeher @dave105010 you are not using laravel-mix right? If you could provide an example using just webpack (as minimal as possible) that would help me out a ton!

Edit: Cleaned up the thread by hiding some comments which provide no new info, please read the posted replies so far before posting.

Hi Siilwyn,

No, we are on Rails 6 framework so not using laravel-mix. This was fixed after updating the dependencies for me. There was a bug on either this or on something else. They released an update a day later. I can't remember the details but updating things fixed this for me.

thecrypticace commented 3 years ago

Do any of you happen to have a lock file (npm or yarn) from a project that is currently broken because it hasn't had it's dependencies updated? I'd love to track down what caused this. :D

dave105010 commented 3 years ago

Hi @Siilwyn and @thecrypticace ,

This was the dependency that caused this issue: https://github.com/NMFR/optimize-css-assets-webpack-plugin/issues/168 It's fixed in 5.0.8 after updating to that dependency to that version, the problem was resolved.

Siilwyn commented 3 years ago

@thecrypticace maybe this has something to do with Sass Comments that contain ! since Sass treats them differently by keeping them in the output even when compressed.

Issue is present with the following files as discovered by kraftner:

Styling:

body {
  /*!*/
  border: 0;
  /*!*/
}

Webpack config:

const mix = require('laravel-mix');

mix.sass('test.scss', 'dist', {
    sassOptions: {
        //outputStyle: 'expanded'
    }
});

Package.json:

{
  "scripts": {
    "build": "mix --production"
  },
  "devDependencies": {
    "laravel-mix": "^6.0.27",
    "resolve-url-loader": "^4.0.0",
    "sass": "^1.38.0",
    "sass-loader": "^12.1.0"
  }
}
thecrypticace commented 3 years ago

Weird yeah I can reproduce it right now with that file. Remove either of the comments lets it build

thecrypticace commented 3 years ago

I can reproduce the problem with just css-declaration-sorter:

import postcss from 'postcss';
import cssDeclarationSorter from 'css-declaration-sorter';

postcss([cssDeclarationSorter()])
  .process(`body{/*!*/border:0;/*!*/}`, { from: undefined })
  .then(result => console.log(result.css) );

The difference here is the lack of newlines. The output is compressed by SASS before getting to CSS declaration sorter.

thecrypticace commented 3 years ago

I tracked it down to this:

const pairedNode = node.prev() ? node.prev() : node.next().next();
if (pairedNode) {
  comments.push({
    'comment': node,
    'pairedNode': pairedNode,
    'insertPosition': 'After',
  });
  node.remove(); // This line
}

On the when processing the 2nd comment (I think — I'm not well versed in how exactly to pull all the relevant info out of the PostCSS node) before removing the comment the pairedNode has a parent. After removing the node the pairedNode's parent becomes undefined.

Siilwyn commented 3 years ago

Oh! Thanks for the reproducible test @thecrypticace, very much appreciated! :) Now onto fixing it!

Siilwyn commented 3 years ago

I am surprised this bug has been present for so long!

Released a fix in 6.1.2.

favri commented 2 years ago

Hi there, i'm having a similar problem to the one reported here.

This snippet: 

`swiper-container-coverflow .swiper-wrapper {
  /* Windows 8 IE 10 fix */
  -ms-perspective: 1200px;
}

/**
 * Swiper 4.4.1
 * Most modern mobile touch slider and framework with hardware accelerated transitions
 * http://www.idangero.us/swiper/
 *
 * Copyright 2014-2018 Vladimir Kharlampidi
 *
 * Released under the MIT License
 *
 * Released on: September 14, 2018
 */
.swiper-container {
  margin: 0 auto;
  position: relative;
  overflow: hidden;
  list-style: none;
  padding: 0;
  /* Fix of Webkit flickering */
  z-index: 1;
}`

Is braking with the same insertAfter error. The problem is with the comment in the middle of the two class declarations. If i erase that, everything is solved.


/**
 * Swiper 4.4.1
 * Most modern mobile touch slider and framework with hardware accelerated transitions
 * http://www.idangero.us/swiper/
 *
 * Copyright 2014-2018 Vladimir Kharlampidi
 *
 * Released under the MIT License
 *
 * Released on: September 14, 2018
 */
Siilwyn commented 2 years ago

@favri are you using version 6.1.2 or later?

favri commented 2 years ago

Hi! I'm using version 6.1.3. with post css and Webpack.

I might have found an other comment with /*! That might be causing the problem too but I thought that was solved in version 6.1.2 😔.

I'm out of home now, but I'll send you some code as soon as I can.

Siilwyn commented 2 years ago

@favri I can't replicate the issue in a test, checkout branch https://github.com/Siilwyn/css-declaration-sorter/tree/issue-170 Would really appreciate a failing test.

dgattey commented 2 years ago

I've got a really complicated dependency chain: gatsby#css-minimizer-webpack-plugin#cssnano#cssnano-preset-default#css-declaration-sorter but basically, Gatsby uses Webpack, which under the hood uses this package. I started having this exact same issue when upgrading from Gatsby 3 to Gatsby 4. With Gatsby 3, I had 6.0.3 transitively installed. With Gatsby 4, the version of css-declaration-sorter got bumped to 6.1.3.

And I can confirm that on 6.1.3, manually removing all instances of /*! in scss files causes my build to work when before I was getting the same TypeError: Cannot read property 'insertAfter' of undefined. So it appears to still be broken even in that version.

The exact comments breaking it were these at the very top of their respective .scss files:

/*!
 * Bootstrap v4.0.0 (https://getbootstrap.com)
 * Copyright 2011-2018 The Bootstrap Authors
 * Copyright 2011-2018 Twitter, Inc.
 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)
 */
/*!
 * Quill Editor v1.3.7
 * https://quilljs.com/
 * Copyright (c) 2014, Jason Chen
 * Copyright (c) 2013, salesforce.com
 */
/*!
 * Bootstrap v4.4.1 (https://getbootstrap.com)
 * Copyright 2011-2019 The Bootstrap Authors
 * Copyright 2011-2019 Twitter, Inc.
 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)
 */
dgattey commented 2 years ago

@Siilwyn Played with it a bit. It's the ! being last on the line that opens the comment. These both work fine:

/*
 !
 * Bootstrap v4.4.1 (https://getbootstrap.com)
 * Copyright 2011-2019 The Bootstrap Authors
 * Copyright 2011-2019 Twitter, Inc.
 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)
 */
/*!*
 * Bootstrap v4.4.1 (https://getbootstrap.com)
 * Copyright 2011-2019 The Bootstrap Authors
 * Copyright 2011-2019 Twitter, Inc.
 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)
 */
Siilwyn commented 2 years ago

@dgattey I really need a test to reproduce the issue.

This works like expected:

import postcss from 'postcss';
import cssDeclarationSorter from 'css-declaration-sorter';
import postcssScss from 'postcss-scss'

postcss([cssDeclarationSorter()])
  .process(`/*!
 * Bootstrap v4.0.0 (https://getbootstrap.com)
 * Copyright 2011-2018 The Bootstrap Authors
 * Copyright 2011-2018 Twitter, Inc.
 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)
 */`, { from: undefined, syntax: postcssScss })
  .then(result =>
    console.log(result.css)
  );
gagegolden00 commented 1 year ago

So I added @apply to my application.tailwind.css file and it made this error

11:44:25 css.1 | TypeError: Cannot read properties of undefined (reading 'insertAfter') 11:44:25 css.1 | at Root.after (/snapshot/tailwindcss/node_modules/postcss/lib/node.js:161:17) 11:44:25 css.1 | at partitionRules (/snapshot/tailwindcss/lib/lib/partitionApplyAtRules.js:49:18) 11:44:25 css.1 | at /snapshot/tailwindcss/lib/lib/partitionApplyAtRules.js:56:9 11:44:25 css.1 | at /snapshot/tailwindcss/lib/processTailwindFeatures.js:34:46 11:44:25 css.1 | at Object.Once (/snapshot/tailwindcss/lib/cli/build/plugin.js:262:19) 11:44:25 css.1 | at LazyResult.runOnRoot (/snapshot/tailwindcss/node_modules/postcss/lib/lazy-result.js:337:23) 11:44:25 css.1 | at LazyResult.runAsync (/snapshot/tailwindcss/node_modules/postcss/lib/lazy-result.js:393:26) 11:44:25 css.1 | at async Object.watch (/snapshot/tailwindcss/lib/cli/build/plugin.js:375:13) 11:44:25 css.1 | at async build (/snapshot/tailwindcss/lib/cli/build/index.js:47:9)

Don't know if this helps or not but just in case here it is, removed it and it works fine. I'm not too keen on these things for now.

Siilwyn commented 1 year ago

@gagegolden00 could you give a reproducible example (or the full file) that causes the error?

gagegolden00 commented 1 year ago

yea just add @apply to your application.tailwind.css file.. like so And then you have the error. Like I said I'm not sure how helpful this is but I had this problem and saw it was giving some people some trouble as well thought I'd give whatever input I had. I not the greatest at this, I'm very new to rails and all of the gems, etc... Not sure what I was doing with that but maybe it will help figure this out.

@tailwind base; @tailwind components; @tailwind utilities; @apply

/*

@layer components { .btn-primary { @apply py-2 px-4 bg-blue-200; } }

*/