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

Process CSS in SVGs #6743

Merged
merged 19 commits into from
Aug 28, 2021
Merged

Process CSS in SVGs #6743

merged 19 commits into from
Aug 28, 2021

Conversation

thewilkybarkid
Copy link
Contributor

↪️ Pull Request

SVGs can include CSS, this will see it processed.

Follows #6595 (currently unmerged, so those changes appear here too) and refs #2901 (comment).

Leaving in draft for the moment as there's definitely more tests to add (and probably changes to make).

💻 Examples

A SVG with CSS will have it processed, so:

<svg viewBox="0 0 240 80" xmlns="http://www.w3.org/2000/svg">
  <style>
    @import './test.css';
  </style>
  <text>Should be red</text>
</svg>

becomes

<svg viewBox="0 0 240 80" xmlns="http://www.w3.org/2000/svg">
  <style>
    :root {
      fill: red
    }
  </style>
  <text>Should be red</text>
</svg>

🚨 Test instructions

Integration test included.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@height
Copy link

height bot commented Aug 19, 2021

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@thewilkybarkid
Copy link
Contributor Author

thewilkybarkid commented Aug 19, 2021

The test failures seem to be that the packager (i.e. PostHTML) doesn't produce self-closing tags, which I assume the optimizer would handle anyway? Relates to #6743 (comment), in that it's probably better to compare XML documents rather than strings.

@thewilkybarkid
Copy link
Contributor Author

thewilkybarkid commented Aug 19, 2021

Found some conflicts with SVGO (namely plugins that use computeStyle, e.g. removeHiddenElems), hitting Empty sub-selector.

(Had to turn off removeHiddenElems, minifyStyles, convertPathData and mergePaths to get it to build.)

@thewilkybarkid
Copy link
Contributor Author

thewilkybarkid commented Aug 19, 2021

SVGO's inlineStyles seems to be problematic too, some selectors not working when used. Seems to be an SVGO problem (svg/svgo#1359).

@thewilkybarkid thewilkybarkid marked this pull request as ready for review August 25, 2021 20:34
@devongovett devongovett merged commit f30f056 into parcel-bundler:v2 Aug 28, 2021
@devongovett
Copy link
Member

Thanks!

@thewilkybarkid thewilkybarkid deleted the svg-css branch August 28, 2021 20:35
lettertwo added a commit that referenced this pull request Sep 10, 2021
* v2: (69 commits)
  Use yarnpkg registry for all the things (#6908)
  Fix issue about loading configs from tsconfig.json (#6881)
  Add `@section` to Compressor type (#6885)
  Parcel API improvements (#6866)
  Use stream-browserify for polyfilling instead (#6863)
  Add support for compressor plugins (#6776)
  Support for inline style attributes and inline scripts in SVG (#6797)
  Bump less (#6852)
  Make sure (non-React) SVGs are in separate bundles (#6757)
  Add support for injecting manifest into service workers (#6798)
  Fix 'does not export default' error with scope hoisting and url/worklet pipeline (#6803)
  Remaining cargo clippy fixes (#6829)
  Lazily install sharp only when needed (#6816)
  More cargo clippy suggestions (#6811)
  Don't emit runtime manifest for inline child bundles (#6807)
  Apply suggestions from rust-clippy (#6256)
  Fix add directoryExists function to matchFiles call. (#6784)
  Upgrade to Flow 158 (#6763)
  Revert "added cio headers (#6404)" (#6789)
  Process CSS in SVGs (#6743)
  ...
lettertwo added a commit that referenced this pull request Sep 10, 2021
* v2: (38 commits)
  Use yarnpkg registry for all the things (#6908)
  Fix issue about loading configs from tsconfig.json (#6881)
  Add `@section` to Compressor type (#6885)
  Parcel API improvements (#6866)
  Use stream-browserify for polyfilling instead (#6863)
  Add support for compressor plugins (#6776)
  Support for inline style attributes and inline scripts in SVG (#6797)
  Bump less (#6852)
  Make sure (non-React) SVGs are in separate bundles (#6757)
  Add support for injecting manifest into service workers (#6798)
  Fix 'does not export default' error with scope hoisting and url/worklet pipeline (#6803)
  Remaining cargo clippy fixes (#6829)
  Lazily install sharp only when needed (#6816)
  More cargo clippy suggestions (#6811)
  Don't emit runtime manifest for inline child bundles (#6807)
  Apply suggestions from rust-clippy (#6256)
  Fix add directoryExists function to matchFiles call. (#6784)
  Upgrade to Flow 158 (#6763)
  Revert "added cio headers (#6404)" (#6789)
  Process CSS in SVGs (#6743)
  ...
lettertwo added a commit that referenced this pull request Sep 11, 2021
* bdo/number-edgetypes: (70 commits)
  Use yarnpkg registry for all the things (#6908)
  Fix issue about loading configs from tsconfig.json (#6881)
  Add `@section` to Compressor type (#6885)
  Parcel API improvements (#6866)
  Use stream-browserify for polyfilling instead (#6863)
  Add support for compressor plugins (#6776)
  Support for inline style attributes and inline scripts in SVG (#6797)
  Bump less (#6852)
  Make sure (non-React) SVGs are in separate bundles (#6757)
  Add support for injecting manifest into service workers (#6798)
  Fix 'does not export default' error with scope hoisting and url/worklet pipeline (#6803)
  Remaining cargo clippy fixes (#6829)
  Lazily install sharp only when needed (#6816)
  More cargo clippy suggestions (#6811)
  Don't emit runtime manifest for inline child bundles (#6807)
  Apply suggestions from rust-clippy (#6256)
  Fix add directoryExists function to matchFiles call. (#6784)
  Upgrade to Flow 158 (#6763)
  Revert "added cio headers (#6404)" (#6789)
  Process CSS in SVGs (#6743)
  ...
lettertwo added a commit that referenced this pull request Sep 13, 2021
* v2: (38 commits)
  Use yarnpkg registry for all the things (#6908)
  Fix issue about loading configs from tsconfig.json (#6881)
  Add `@section` to Compressor type (#6885)
  Parcel API improvements (#6866)
  Use stream-browserify for polyfilling instead (#6863)
  Add support for compressor plugins (#6776)
  Support for inline style attributes and inline scripts in SVG (#6797)
  Bump less (#6852)
  Make sure (non-React) SVGs are in separate bundles (#6757)
  Add support for injecting manifest into service workers (#6798)
  Fix 'does not export default' error with scope hoisting and url/worklet pipeline (#6803)
  Remaining cargo clippy fixes (#6829)
  Lazily install sharp only when needed (#6816)
  More cargo clippy suggestions (#6811)
  Don't emit runtime manifest for inline child bundles (#6807)
  Apply suggestions from rust-clippy (#6256)
  Fix add directoryExists function to matchFiles call. (#6784)
  Upgrade to Flow 158 (#6763)
  Revert "added cio headers (#6404)" (#6789)
  Process CSS in SVGs (#6743)
  ...
lettertwo added a commit that referenced this pull request Sep 13, 2021
* bdo/number-edgetypes: (70 commits)
  Use yarnpkg registry for all the things (#6908)
  Fix issue about loading configs from tsconfig.json (#6881)
  Add `@section` to Compressor type (#6885)
  Parcel API improvements (#6866)
  Use stream-browserify for polyfilling instead (#6863)
  Add support for compressor plugins (#6776)
  Support for inline style attributes and inline scripts in SVG (#6797)
  Bump less (#6852)
  Make sure (non-React) SVGs are in separate bundles (#6757)
  Add support for injecting manifest into service workers (#6798)
  Fix 'does not export default' error with scope hoisting and url/worklet pipeline (#6803)
  Remaining cargo clippy fixes (#6829)
  Lazily install sharp only when needed (#6816)
  More cargo clippy suggestions (#6811)
  Don't emit runtime manifest for inline child bundles (#6807)
  Apply suggestions from rust-clippy (#6256)
  Fix add directoryExists function to matchFiles call. (#6784)
  Upgrade to Flow 158 (#6763)
  Revert "added cio headers (#6404)" (#6789)
  Process CSS in SVGs (#6743)
  ...
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.

3 participants