Treeshake Icon contents

[panoply]

Hey @vrimar

The feather icon pack contributes 50kb of weight to distribution bundles, I'm having trouble tree-shaking those imports. The JSON Mapping of svg contents provided via feather seems the cause. Providing Icons contents as a series of exports would allow svg contents to be cherry picked when bundling for production, I noticed the current approach it to map icon contents to property value but mapping to a vnode would be just as effective, eg:

// some defaults
const some defaults = { viewBox:'0 0 24 24' }

export const icon = (attrs = {}) => m('svg', { ...attrs, ...defaults }, m.trust('d="M0 27.73v27."'))
export const another_icon = (attrs = {}) => m('svg', { ...attrs, ...defaults }, m.trust('d="M0 27.73v27."'))

Are PRs welcome on the project?

[vrimar]

This was actually planned for the 1.0 release.. Ideally I'd like to keep the old usage along with the ability to import single icon components to support tree shaking. It would be a breaking change for both the Button and MenuItem components:

Old:

import { Icons } from "construct-ui"

m(Button, { iconLeft: Icons.USER });

New:

import { UserIcon } from "construct-ui"

m(Button, { iconLeft: m(UserIcon) });

The newer way makes more sense, simply importing the Button component brings along 50kb of Icon data which cannot be tree shaken properly.

PRs are definitely welcome, if you have any ideas let me know.

[panoply]

Each bundler handles tree-shaking differently. One way to combat a breaking change would be to generate each icon as its own file which default exports a vnode and expose them to files[] in package but that would require developers to individually import icons, eg:

import UserIcon  from "construct-ui/user-icon"

m(Button, { iconLeft: m(UserIcon) });

This is similar to how you'd treeshake lodash module when bundling with Rollup before passing to Terser but it doesn't seem fitting for construct and better applied in monorepos that modularize these implementations and leverage @ orgs on the NPM registry. Ideally you'd want each Icon available on the module for convenience sake.

The newer way makes more sense, simply importing the Button component brings along 50kb of Icon data which cannot be tree shaken properly.

I don't think I follow? Do you mean using Button will still result in the iconset being pulled in?

[vrimar]

I don't think I follow? Do you mean using Button will still result in the iconset being pulled in?

Yeah, sorry for the confusion, currently Button imports the Icon component which in turn imports the icon/generated/IconContents.ts.

The iconLeft and iconRight attrs expect an IconName string to be passed and that's why I mentioned it being a breaking change if you end up changing it to accepting vnodes.

Let me look into this more, I'll come up with something when I have time.