fix: avoid purging class used in v-bind:class={}

miyanokomiya commented 5 years ago

Current regex of default extractor cannot get class names in syntax of v-bind:class="{}" correctly.

'<div :class="{ binded: true }" />'.match(/[A-z0-9-:\\/]+/g)

So, the class binded will be purged.

manniL commented 5 years ago

Hey :wave:

First, please use bound instead of binded :relaxed:
Second: would the tests fail right now without the changes you've introduced? Third: this might be a breaking change.

miyanokomiya commented 5 years ago

First, please use bound instead of binded

OK, I will

Second: would the tests fail right now without the changes you've introduced?

Yes, some tests fail when a original regex is used.

 FAIL  test/module.test.js (88.474s)
  nuxt-purgecss
    webpack
      ✕ extract and purge css by default (18888ms)
      ✓ don't show webpack error message in dev (8768ms)
      ✓ globally disable module (8290ms)
      ✕ define custom options for css lookup (concatenating) (8777ms)
      ✓ define custom function options for css lookup (overriding) (8236ms)
    postcss
      ✕ purge css by default (8483ms)
      ✓ globally disable module (8245ms)
      ✕ define custom options for css lookup (concatenating) (8485ms)
      ✓ define custom function options for css lookup (overriding) (8241ms)

webpack

● nuxt-purgecss › webpack › extract and purge css by default

expect(received).toMatch(expected)

Expected value to match:
  ".binded"
Received:
  ".ymca{text-align:center}"

  39 |       expect(testCSS).not.toMatch('.abc')
  40 |       expect(testCSS).toMatch('.ymca')
> 41 |       expect(testCSS).toMatch('.binded')
     |                       ^
  42 |     })
  43 |
  44 |     test('don\'t show webpack error message in dev', async () => {

  at Object.toMatch (test/module.test.js:41:23)

postcss

● nuxt-purgecss › postcss › purge css by default

expect(received).toMatch(expected)

Expected value to match:
  ".binded"
Received:
  "(window.webpackJsonp=window.webpackJsonp||[]).push([[2],{130:function(t,e,n){var s=n(132);\"string\"==typeof s&&(s=[[t.i,s,\"\"]]),s.locals&&(t.exports=s.locals);(0,n(34).default)(\"2ecdafb5\",s,!1,{})},131:function(t,e,n){\"use strict\";var s=n(130);n.n(s).a},132:function(t,e,n){(t.exports=n(33)(!1)).push([t.i,\"\\n.ymca{text-align:center\\n}\",\"\"])},133:function(t,e,n){\"use strict\";n.r(e);var s=function(){var t=this.$createElement,e=this._self._c||t;return e(\"div\",[e(\"h1\",[this._v(\"Test\")]),this._v(\" \"),e(\"test\")],1)};s._withStripped=!0;var i=function(){var t=this.$createElement,e=this._self._c||t;return e(\"div\",{staticClass:\"ymca\"},[e(\"div\",{class:{binded:!0}},[this._v(\"\\n    Test\\n  \")])])};i._withStripped=!0;n(131);var l=n(18),a=Object(l.a)({},i,[],!1,null,null,null);a.options.__file=\"Test.vue\";var c={components:{Test:a.exports}},r=Object(l.a)(c,s,[],!1,null,null,null);r.options.__file=\"index.vue\";e.default=r.exports}}]);"

  123 |       expect(testCSS).not.toMatch('.abc')
  124 |       expect(testCSS).toMatch('.ymca')
> 125 |       expect(testCSS).toMatch('.binded')
      |                       ^
  126 |     })
  127 |
  128 |     test('globally disable module', async () => {

  at Object.toMatch (test/module.test.js:125:23)

Third: this might be a breaking change.

I'm worry about it too. This change may make purging moderate. On the other hand, current extractor purges essential selectors used in v-bind:class="{}". If it is too difficult to accept this breaking soon, it may be nice to add doc that explain the limitation using with v-bind:class="{}"

miyanokomiya commented 5 years ago

https://medium.com/@kyis/vue-tailwind-purgecss-the-right-way-c70d04461475 This article must be better than my explanations😅

manniL commented 5 years ago

Regarding a breaking change: After checking, I think it might only lead to "more" classes not being purged (so a few false positives). On the other hand, it'll save Vue users from issues, so I'm glad to accept this.

I want to release v1 soon anyway, so even if it's considered breaking, it'll be in soon.

miyanokomiya commented 5 years ago

My code is conservative to reduce breakings as much as possible. If you can accept more possibility of breakings, it may be better to use regex /[A-Za-z0-9-_/:]*[A-Za-z0-9-_/]+/g that is introduced in that article.

manniL commented 5 years ago

Feel free to adapt it then :relaxed:

miyanokomiya commented 5 years ago

I have adpoted. If you have other thought for a default extractor, feel free to close this PR and improve yourself😀

manniL commented 5 years ago

I'll go ahead and introduce more DX changes from the linked post :relaxed:
Thank you!

studnitz commented 4 years ago

https://github.com/Developmint/nuxt-purgecss/pull/79 should fix this

manniL commented 4 years ago

Thanks for the contribution! Closing here.

Developmint / nuxt-purgecss

fix: avoid purging class used in v-bind:class={} #54