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

feat: added @parcel/css support #154

Merged
merged 1 commit into from
Jan 18, 2022
Merged

Conversation

alexander-akait
Copy link
Member

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

fixes #153

Breaking Changes

No

Additional Info

  • prefixes are broken and do not work
  • minification is broken for special IE10/11 properties (i.e. *prop: value)
  • collapsing properties are broken in some cases
  • escaped things are broken

@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Merging #154 (78c3dc9) into master (f558512) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #154      +/-   ##
==========================================
+ Coverage   95.86%   95.88%   +0.01%     
==========================================
  Files           3        3              
  Lines         266      267       +1     
  Branches      107      107              
==========================================
+ Hits          255      256       +1     
  Misses         11       11              
Impacted Files Coverage Δ
src/utils.js 91.66% <ø> (ø)
src/index.js 98.10% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f558512...78c3dc9. Read the comment docs.

@alexander-akait alexander-akait merged commit 5e5fe51 into master Jan 18, 2022
@alexander-akait alexander-akait deleted the feat-added-parcel-css branch January 18, 2022 11:55
@devongovett
Copy link

This is cool to see! Thanks for doing this.

minification is broken for special IE10/11 properties (i.e. *prop: value)

Still deciding what to do about this. See parcel-bundler/lightningcss#39.

Could you explain more about the other issues you ran into? Were there broken code samples I could look at?

@alexander-akait
Copy link
Member Author

@devongovett

  • prefixes are broken and do not work

I tried to set chrome: 4 and use border-radius and unfortunately, no prefixes, tried it for transition and other properties popular properties, and prefixes just not outputted for all cases (also tried different targets), tried to use browserslist_to_target, it returns something weird (like version) and it still not work

  • collapsing properties are broken in some cases

I run it on cssnano codebase and there a some cases, most around prefixes and normal writing, sorry, I don't have time to fully investigate it, try to adapt cssnano tests (quite possible there are cssnano bugs too), but they are mostly edge cases

escaped things are broken

I was wrong to use here broken, sorry, for example:

    content: "a\
b";

Can be collapsed in one string, good examples:

a::before {
    content: "This string is demarcated by double quotes.";
    content: 'This string is demarcated by single quotes.';
    content: "This is a string with \" an escaped double quote.";
    content: "This string also has \22 an escaped double quote.";
    content: 'This is a string with \' an escaped single quote.';
    content: 'This string also has \27 an escaped single quote.';
    content: "This is a string with \\ an escaped backslash.";
    content: "This string also has \22an escaped double quote.";
    content: "This string also has \22  an escaped double quote.";
    content: "This string has a \Aline break in it.";
    content: "A really long \
awesome string";
    content: ";'@ /**/\"";
    content: '\'"\\';
    content: "a\
b";
    content: "a\
b";
    content: "a\
b";
    content: "a\b";
    content: "a\
\
\
\\
b";
    content: 'a\62 c';
}

a[title="a not s\
o very long title"] {
    color: red;
}
a[title="a not so very long title"] {
    color: red;
}

@devongovett
Copy link

Thanks for the info!

I see prefixes for border-radius working in the playground, but I'll try it out with the webpack plugin.

Haven't handled the content property yet, but those are good examples.

@alexander-akait
Copy link
Member Author

alexander-akait commented Jan 18, 2022

@devongovett Maybe something wrong between JS and Rust, because I tried the same and no prefixes, usage:

new CssMinimizerPlugin({
  minimizerOptions: {
    targets: {
      chrome: 4,
    },
    drafts: {
      nesting: true,
    },
  },
  minify: [CssMinimizerPlugin.parcelCssMinify],
}).apply(compiler);

Example without plugin:

const parcelCss = require("@parcel/css");

async function test() {
  const parcelCssOptions = {
    minify: true,
    targets: { chrome: 4 },
    drafts: { nesting: true },
    sourceMap: true,
    filename: "foo.css",
    code: Buffer.from(".foo {border-radius: 24px;}"),
  };

  const result = await parcelCss.transform(parcelCssOptions);
  console.log(result.code.toString());
  // .foo{border-radius:24px}
}

test();

@devongovett
Copy link

Ah, I see the issue. For performance, targets are represented using a 24 bit number to represent major.minor.patch versions, so you have to use some bit shifting to make it work. See https://github.com/parcel-bundler/parcel-css#from-node. Maybe we can improve the API a bit to make this easier to understand, or add a util function for creating these versions.

targets: {
  chrome: 4 << 16
}

@alexander-akait
Copy link
Member Author

@devongovett Yep, feel free to improve this, I think it is out of scope plugin, don't think we should do it here

@devongovett
Copy link

Yup, definitely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request - parcel-css as custom minifier
2 participants