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

Icons Package: Add build process #38850

Open
jasmussen opened this issue Feb 16, 2022 · 5 comments
Open

Icons Package: Add build process #38850

jasmussen opened this issue Feb 16, 2022 · 5 comments
Labels
Needs Dev Ready for, and needs developer efforts Needs Figma Update Needs an update to Figma for design purposes [Package] Icons /packages/icons [Type] Enhancement A suggestion for improvement.

Comments

@jasmussen
Copy link
Contributor

As the project grows, so does the need to simplify the contribution flow for new icons. In that light, we should consider adding a build step to the Icons package. It could work like this:

  • The src folder of the packages/icons becomes a gitignored build folder
  • The new src folder should be just a list of raw SVGs exported from the WordPress Design Library Figma.
  • We need a new script there to transform from one to another, it could live in packages/icons/build.js.

For the build script, we can potentially take inspiration from this build process.

There are a few steps that need to be taken to improve the Figma file itself, to enable this flow:

  • Filenames between Figma components and published icons in the package need to be synced up, a few have drifted.
  • Some icons were added to the package which should probably not be there (see also).
  • Icons need to be flattened in Figma so they export as a single path.

The Figma changes can happen independently of a build step. Here's a zip file containing raw SVG versions of the existing published library, as well as a first draft of a new README file:

icons-package.zip

@jasmussen jasmussen added [Type] Enhancement A suggestion for improvement. Needs Dev Ready for, and needs developer efforts Needs Figma Update Needs an update to Figma for design purposes [Package] Icons /packages/icons labels Feb 16, 2022
@mirka
Copy link
Member

mirka commented Apr 23, 2024

@tbradsha Since you're interested in working on this, here's what I think could be the success criteria for the minimum viable first iteration:

  • The canonical svg files are committed to the repo instead of the js files we currently have in src/library.
  • npm run build builds without errors. Our codebase contains plenty of imports from @wordpress/icons, so if all that still builds, we can assume we're good.
  • If we run npm run dev and add a new svg or make a change to an existing svg, that should automatically be reflected in the built package.

(In future iterations, we could add more export types, for example so the original svg files can be directly consumed with import { someIcon } from '@wordpress/icons/svg.)

As for where to start, the entry point of the repo's build process is this build.js file, which then delegates work to build-worker.js. I think the icon processing will need to happen somewhere in that vicinity.

Is that enough info to get started?

@tbradsha
Copy link

@mirka Thanks! Just to make sure we're on the same page for the initial scope:

  • The contents of src/library should be generated by the build and should be removed from the repo.
  • The only contents committed to this package would be SVG files (pre-cleaned, stored in ./svg) and some tooling scripts (for building SVGs to the React, stored in ./scripts or ./tools).
  • I'm a bit unclear what the difference between npm run dev and npm run build would be in this package. In both instances I'd expect it to read the SVGs and generate the interim JS files that go into the ./src/library folder (as well as generating a new ./src/index.js file). Can you clarify if they should do something different?

@mirka
Copy link
Member

mirka commented Apr 24, 2024

  • The contents of src/library should be generated by the build and should be removed from the repo.
  • I'm a bit unclear what the difference between npm run dev and npm run build would be in this package. In both instances I'd expect it to read the SVGs and generate the interim JS files that go into the ./src/library folder (as well as generating a new ./src/index.js file). Can you clarify if they should do something different?

Ideally, they are auto-generated into the packages/icons/build* folders. (Those build folders are already gitignored.) Nothing under src/ should be auto-generated. I'd first attempt to do it with Webpack, for example we'd have a static index.js file like this:

// src/index.js
export someIcon from './library/some-icon.svg';
export anotherIcon from './library/another-icon.svg';

and add some webpack config for the icons package so SVG files are loaded as React components, using something like @svgr/webpack.

By convention, npm run build is a build script that builds the package and exits when done. npm run dev is a build script that continues running until explicitly terminated, during which it watches for src file changes so they can be continuously rebuilt as necessary. I only included it as part of the success criteria because we might need to add some logic to specifically watch for SVG file changes.

  • The only contents committed to this package would be SVG files (pre-cleaned, stored in ./svg) and some tooling scripts (for building SVGs to the React, stored in ./scripts or ./tools).

No strong opinion on where the tooling scripts live. It could either live in bin/packages/ or packages/icons/, but in any case we can move that easily.

@tbradsha
Copy link

Ideally, they are auto-generated into the packages/icons/build* folders.

Great, so skip src altogether when generating things. I just wanted to make sure, as all the build* and src folders were distributed. 👍

I only included it as part of the success criteria because we might need to add some logic to specifically watch for SVG file changes.

Ah, makes sense.

@jasmussen
Copy link
Contributor Author

A note from one of the PRs:

For future ideas, feel free to take a look at this repo: https://github.com/Automattic/jetpack/tree/trunk/projects/js-packages/social-logos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Ready for, and needs developer efforts Needs Figma Update Needs an update to Figma for design purposes [Package] Icons /packages/icons [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

3 participants