pascalduez / postcss-apply

PostCSS plugin enabling custom properties sets references
The Unlicense
164 stars 12 forks source link

Using this after postcss-nested breaks the plugin #16

Open dan-gamble opened 7 years ago

dan-gamble commented 7 years ago

The problem it causes is:

On this line https://github.com/pascalduez/postcss-apply/blob/master/src/visitor.js#L16, normally it looks like: --toolbar-theme: but with postcss-nested it looks like: :root --test-theme: null.

Moving it before postcss-nested fixed it for me so might be worth mentioning that somewhere in the README :)

pascalduez commented 7 years ago

normally it looks like: --toolbar-theme: but with postcss-nested it looks like: :root --test-theme: null.

hi,

could you please but a little bit more specific/clear (samples welcome).

dan-gamble commented 7 years ago

So i put console.log(matches) after https://github.com/pascalduez/postcss-apply/blob/master/src/visitor.js#L16

When using this plugin matches returns things like:

--toolbar-theme
--another-property

But if this plugin is used after postcss-nested

matches returns:

:root --test-theme:
:root --another-property

This makes these properties null so we hit: https://github.com/pascalduez/postcss-apply/blob/master/src/visitor.js#L18

Does that help at all?

pascalduez commented 7 years ago

All right, still a bit weird, but I'll try to reproduce and see what we can do to make both plugins play together. But the issue looks more on the postcss-nested part at a first glance.

Side not: what about postcss-nesting instead?

dan-gamble commented 7 years ago

I've not tried postcss-nesting myself so couldn't really say. And i'd say it probably is more postcss-nested's fault than yours :)

ritasilvasousa commented 7 years ago

Hi,

I'm having a similar issue. I'm using postcss-apply v0.6 because I need to have sets in JS. However I also need to create sets in css and do something like this:

:root {
    --bar {
        background: #000000;
        color: #ffffff;

        &-title {
            color: #f8f8f8;
        }
    }
}

.foo {

    @apply --bar;

    &-title {
        height: 30rem;
    }
}

The problem is that it won't render the nested title properties. I guess this is a issue between the apply and the nested plugins, because if I manually update the cssnext to use apply in v0.6 it all works great.

Can you help me workaround this issue until cssnext updates the apply plugin?

This is my implementation:

Package:

"postcss-apply": "^0.6.1",
"postcss-cssnext": "^2.x.x",

Postcss Plugins:

plugins: [
             require('postcss-import'),
            require('postcss-reporter'),
            require('postcss-mixins'),
            require('postcss-cssnext')({
                features: {
                    customMedia: {
                        extensions:  mediaqueries,
                    },
                    customProperties: {
                        variables: variables,
                    },
                    autoprefixer: {
                        browsers: ['last 2 versions'],
                        remove: false,
                    },
                    calc: {
                        preserve: true,
                    },
                    applyRule: false,
                    rem: false,
                },
            }),
            require('postcss-apply')({
                sets: sets,
            }),
            require('postcss-nested'),
        ],

Thanks

pascalduez commented 7 years ago

Hi @ritasousa-farfetch,

I'll try to run some tests, see what we can do. But there's some crucial points to understand here: postcss-cssnext and postcss-apply aims at following the standard specs as close as possible. Although with a few convenience features likes being able to declare custom sets or props from js.

As far as I can tell, custom property sets name interpolation is not part of any spec.

--bar {
  color: #ffffff;

  &-title {
    color: #f8f8f8;
   }
}

So bascally, this won't be added to the postcss-apply plugin.

Not to shirk reponsabilities, but at a first glance this looks more like an issue for postcss-nested, that plugin should be able to construct the new custom property set name. In such case it should be run before the postcss-apply plugin in your setup. Did you tried that?

To be honest, what's so terrible about the following?


:root {
  --bar {
    background: #000000;
    color: #ffffff;
  }
  --title {
    color: #f8f8f8;
  }
}

.foo {
  @apply --bar;
}
.foo-title {
  @apply --bar;
  @apply --title;
  height: 30rem;
}
dan-gamble commented 7 years ago

I agree i don't think

:root {
    --bar {
        background: #000000;
        color: #ffffff;

        &-title {
            color: #f8f8f8;
        }
    }
}

Will work, you can use: https://github.com/andyjansson/postcss-sassy-mixins like i do with postcss-apply and then use something like:

@mixin bar {
  background: #000000;
  color: #ffffff;

  &-title {
    color: #f8f8f8;
  }
}
ritasilvasousa commented 7 years ago

Thanks for your quick answers.

It worked before disabling the applyRule in the cssnext and requiring apply separately to use v0.6. You can test it in the cssnext playground, the exact same css renders to:

.foo {
    background: #000000;
    color: #ffffff
}

.foo-title {
    color: #f8f8f8
}

.foo-title {
    height: 480px;
    height: 30rem
}

I've tried to run nested before the apply but it doesn't even recognise the set.

(Emitted value instead of an instance of Error) postcss-apply: 

/path/styles.css:20:5: No custom property set declared for `bar`.
pascalduez commented 7 years ago

@ritasousa-farfetch Okay, I see.

Did you try just upgrading the postcss-apply plugin in your deps but not running it again after the cssnext one (in plugin list).

So basically just a npm i postcss-apply@latest.

So back at what I supposed your config was:

plugins: [
  require('postcss-import'),
  require('postcss-reporter'),
  require('postcss-mixins'),
  require('postcss-cssnext')({
    features: {
      customMedia: {
        extensions:  mediaqueries,
      },
      customProperties: {
        variables: variables,
      },
      autoprefixer: {
        browsers: ['last 2 versions'],
        remove: false,
      },
      calc: {
        preserve: true,
      },
      apply: {
        sets: sets,
      },
      rem: false,
    },
  }),
],
pascalduez commented 7 years ago

Also the plugin postcss-cssnext is using is postcss-nesting, not postcss-nested.

ritasilvasousa commented 7 years ago

Thanks so much for your help.

The last solution would work but not on a production environment, as the dependencies are all reinstalled with the tags.

Maybe not using nested sets is the way to go, as you said they're not in the standard specs.

pascalduez commented 7 years ago

@ritasousa-farfetch Okay, I'll try to make cssnext update the dependency ;)

For the prod env, maybe a postinstall script in your package.json?

"scripts": {
  "postinstall": "npm i postcss-apply@latest"
}