feat(octicons_react): replace usage of `dangerouslySetInnerHTML`
This is a follow-up to the experiment in: https://github.com/primer/octicons/pull/837
Closes https://github.com/primer/octicons/issues/796
What's changed
At a high-level, this PR changes a component from being defined as:
function AccessibilityIcon(props) {
var svgDataByHeight = { "16": { "width": 16, "path": "<path fill-rule=\"evenodd\" d=\"M9.923 5.302a3 3 0 10-3.847 0A2.713 2.713 0 005.9 5.5H2A.75.75 0 002 7h3.3l-.578 5.163-.362 2.997a.75.75 0 101.49.18L6.132 13h3.736l.282 2.34a.75.75 0 101.49-.18l-.362-2.997L10.7 7H14a.75.75 0 000-1.5h-3.899a2.697 2.697 0 00-.178-.198zM9.5 3a1.5 1.5 0 11-3 0 1.5 1.5 0 013 0zm-.3 4.073l.495 4.427h-3.39l.496-4.427a1.207 1.207 0 012.398 0z\"></path>" } };
return React.createElement('svg', getSvgProps(_extends({}, props, { svgDataByHeight: svgDataByHeight })));
}
AccessibilityIcon.defaultProps = {
className: 'octicon octicon-accessibility',
size: 16,
verticalAlign: 'text-bottom'
};
To the following:
const AccessibilityIcon = /*#__PURE__*/ createIconComponent("AccessibilityIcon", "octicon octicon-accessibility", () => ({
"16": {
"width": 16,
"path": /*#__PURE__*/React.createElement("path", {
fillRule: "evenodd",
d: "M9.923 5.302a3 3 0 10-3.847 0A2.713 2.713 0 005.9 5.5H2A.75.75 0 002 7h3.3l-.578 5.163-.362 2.997a.75.75 0 101.49.18L6.132 13h3.736l.282 2.34a.75.75 0 101.49-.18l-.362-2.997L10.7 7H14a.75.75 0 000-1.5h-3.899a2.697 2.697 0 00-.178-.198zM9.5 3a1.5 1.5 0 11-3 0 1.5 1.5 0 013 0zm-.3 4.073l.495 4.427h-3.39l.496-4.427a1.207 1.207 0 012.398 0z"
})
}
}));
Under the hood, createIconComponent returns the AccessibilityIcon component. This function brings over the existing getSvgProps behavior and bakes it into the returned component.
How it works
The default lib/build/data.json file now includes an AST of the SVG asset. The icon builder in lib/octicons_react converts this svg AST to JS which is included in the svgData argument of createIconComponent.
createIconComponent creates the component used by React and inlines the existing logic from getSvgData into the component.
The notable change at this point is that the path value is no longer a string but is instead a JSXElement (or JSXFragment) which can be set as the children value of the <svg> element.
Testing & Reviewing
I updated the Next.js project to work with the package format so it should allow a preview of the new icon components!
⚠️ No Changeset found
Latest commit: 999060f0287f746d17ee740ccf7d0b5195c1a882
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Note: we should make sure https://github.com/primer/octicons/pull/841 is merged before this PR in order to make sure no breaking changes occur
@colebemis good call on adding that individual test for tree shaking, it seems like React.Fragment is not considered pure (even when used in a pure React.createElement call) and is causing icons that use it to be included 🤔
I updated the approach to include two things:
- A
/*#__PURE__*/annotation on thecreateIconComponentfunction call - A getter for the
svgDataByHeightobject
The getter allows things in that top scope to be tree-shaken since the impure React.Fragment now is only accessed if the function is called.
It seems like the size of the export reduced by ~1kB so tree-shaking is working as-intended with no public API changes (phew 😅)