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

Add SVGR support to wp-scripts #18243

Merged
merged 6 commits into from
Jan 8, 2020
Merged

Add SVGR support to wp-scripts #18243

merged 6 commits into from
Jan 8, 2020

Conversation

mkaz
Copy link
Member

@mkaz mkaz commented Nov 1, 2019

Description

This PR adds @svgr/webpack plugin to compile SVG icons to React Components during the build step. This allows for loading an svg direct from a file and using as an component, for example:

import MyIcon from './icon.svg'
...
edit: () => {
    <>
        <MyIcon />
    </>
}

Related to #14628

How has this been tested?

In a separate project using wp-scripts build added a svg file and loaded and displayed.

Types of changes

Enhancement to wp-scripts

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@mkaz mkaz added the [Status] In Progress Tracking issues with work in progress label Nov 1, 2019
@mkaz mkaz added the [Package] Scripts /packages/scripts label Nov 1, 2019
@mkaz
Copy link
Member Author

mkaz commented Nov 1, 2019

@gziolo I got this to work here - but struggled with getting it to work with the main Gutenberg webpack.config.js. It is relatively straight-forward to get to work here, as you can see by the change. However, due to the nature of how files get copied and built in the main project/packages/modules I wasn't able to get working there yet.

@gziolo
Copy link
Member

gziolo commented Nov 4, 2019

I don't have experience with this webpack plugin so I can't help much here. You can ask in #core-js channel for help. I also anticipate a bit of a challenge in terms of using it inside Gutenberg. This comes from the fact that we use only Babel to generate code shipped to npm so this would create invalid imports which Node can't process out of the box. At the same time, Jest will also need a custom handler to make such code execute without errors, we have something similar for CSS imports:
https://github.com/WordPress/gutenberg/blob/master/packages/jest-preset-default/jest-preset.json#L3

Aside: we don't use CSS imports anymore in the web part, but I think React Native does that :)

@gziolo
Copy link
Member

gziolo commented Dec 17, 2019

This reminds me of the addition to the docs introduced by @mor10:

https://github.com/WordPress/gutenberg/tree/master/packages/scripts#extending-the-webpack-config

It had something else integrated, so it would be nice to follow the example. It also means we would have to update this example to use some other loader 😄

@mkaz
Copy link
Member Author

mkaz commented Jan 7, 2020

@gziolo updated with url-loader which was included in @mor10 example.
I switched the extended example to use CSS and Style loaders.

I'm not sure how the changelog and version number should be updated.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how the changelog and version number should be updated.

It's all documented in https://github.com/WordPress/gutenberg/tree/master/packages#maintaining-changelogs. Long story short, you add ## Master section and a section for changes. In this case, it'll be New Features. The version of the package should stay as is until we do next npm release.

packages/scripts/package.json Show resolved Hide resolved
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested it by it looks good as far as I can tell.

It would be great to schedule some testing once it's published to npm.

@mkaz mkaz merged commit 416f1c5 into master Jan 8, 2020
@mkaz mkaz deleted the add/svgr-scripts branch January 8, 2020 20:27
@ellatrix ellatrix added this to the Gutenberg 7.3 milestone Jan 20, 2020
swissspidy added a commit to GoogleForCreators/web-stories-wp that referenced this pull request Feb 6, 2020
Less magic, less dependencies, less security warnings.

See WordPress/gutenberg#18243
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Scripts /packages/scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants