Large front-end bundles? about sanity-plugin-icon-picker HOT 17 CLOSED

@christopherafbjur Created a PR! Let me know your thoughts :)

from sanity-plugin-icon-picker.

thanks @blueputty01 looks like a smart & sharp solution

from sanity-plugin-icon-picker.

Hi @blueputty01! Indeed it doesn't seem like there is much that can be done regarding react-icons. How ever your last proposal is actually something I've been thinking about. Might be something we can implement that could be toggled on with an option. Feel free to create a PR. I have limited time at this point but will try to find time to stay more active with this lib asap 😊

from sanity-plugin-icon-picker.

Hi @christopherafbjur! I'll look into this more and open a PR hopefully next weekend.

Reopening since I think that the link to the PR will be more clear. Thanks again for this plugin!

from sanity-plugin-icon-picker.

@blueputty01 Thank you for this proposal, this was a really neat idea! How ever I was unable to reproduce this using .baseElement when testing in a studio since it was returning the document.body which seem to be the expected behavior (reference).
How ever I could reproduce the same svg string if instead using
render(icon.component()).container.innerHTML. The problem however is that in both scenarios (using the render method) a div will be appended to document.body to which the icon component will be appended (reference).

And in our case this means that each time we select an icon in the picker, a div will be appended to the document.body since it's never removed. Doesn't seem optimal to pollute the DOM like this.

Example of toggling between 3 different icons from the picker

from sanity-plugin-icon-picker.

Checked this briefly and it looks great! Will have a closer look tomorrow @blueputty01

from sanity-plugin-icon-picker.

Closed, this is more of an issue with react-icons not being tree-shakeable.

from sanity-plugin-icon-picker.

What if an SVG was set as a part of the data as well to make this more front-end language-agnostic & avoid large builds?

Eg:

  const setIcon: SearchResultsOnSelectCallback = (icon) => {
    if (selected && icon.name === selected.name) return unsetIcon();

    onChange([
      setIfMissing({
        _type: schemaType.name,
      }),
      set(icon.name, ['name']),
      set(icon.provider, ['provider']),
      set(renderToString(icon.component()), ['svg']), // <-- addition
    ]);

from sanity-plugin-icon-picker.

Tested your proposed solution @blueputty01 but it seems that it requires configuring vite with polyfills which is probably not the job for this plugin (I assume you were thinking of using renderToString method from react-dom/server dependency). But feel free to correct me if I'm wrong :)

Another solution that I tested and that seem to work is:

Plugin

import {format, plugins} from 'pretty-format';
import renderer from 'react-test-renderer';
...
...
const setIcon: SearchResultsOnSelectCallback = (icon) => {
    if (selected && icon.name === selected.name) return unsetIcon()

    const svgString = format(renderer.create(icon.component()).toJSON(), {
      plugins: [plugins.ReactTestComponent],
      printFunctionName: false,
    }).replace(/\s+|\n/gm, ' ')
    // <svg fill=\"none\" height=\"1em\" stroke=\"currentColor\" strokeLinecap=\"round\" strokeLinejoin=\"round\" strokeWidth=\"2\" style={ Object { \"color\": undefined, } } viewBox=\"0 0 24 24\" width=\"1em\" xmlns=\"http://www.w3.org/2000/svg\" > <circle cx=\"12\" cy=\"12\" r=\"10\" /> <polyline points=\"8 12 12 16 16 12\" /> <line x1=\"12\" x2=\"12\" y1=\"8\" y2=\"16\" /> </svg>

    onChange([
      setIfMissing({
        _type: schemaType.name,
      }),
      set(icon.name, ['name']),
      set(icon.provider, ['provider']),
      set(svgString, ['svg']),
    ])


    return setSelected(icon)
  }

Client

import SVG from 'react-inlinesvg';

const sanityResponse = {
    "_type": "iconPicker",
    "name": "arrow-down-circle",
    "provider": "fi",
    "svg": "<svg fill=\"none\" height=\"1em\" stroke=\"currentColor\" strokeLinecap=\"round\" strokeLinejoin=\"round\" strokeWidth=\"2\" style={ Object { \"color\": undefined, } } viewBox=\"0 0 24 24\" width=\"1em\" xmlns=\"http://www.w3.org/2000/svg\" > <circle cx=\"12\" cy=\"12\" r=\"10\" /> <polyline points=\"8 12 12 16 16 12\" /> <line x1=\"12\" x2=\"12\" y1=\"8\" y2=\"16\" /> </svg>"
  }
...
<SVG src={sanityResponse.svg} />

from sanity-plugin-icon-picker.

@christopherafbjur Thanks for taking a look at this! I was indeed thinking of the function from react-dom/server. Your solution is more clean.

Issue:

The sample client component works well in my tests.

Using encodeURIComponent to populate the src attribute on an img tag using the generated SVG markup doesn't seem to work as well.

It seems like the parser gets confused by the curly braces in the style attribute: style={ Object { \"color\": undefined, } }

Proposal:

• A regex expression eg \{.*\} to remove objects from the final string
• Removing the style key/any keys with object type values from the react-test-renderer object before passing it to pretty-format
• Possibly leave it as an upstream/downstream issue...?

Issue example:

For example, the fi alert circle passed into encodeURIComponent produces the error: error on line 1 at column 120: AttValue: " or ' expected

Generated SVG:
<svg fill="none" height="1em" stroke="currentColor" strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" style={ Object { "color": undefined, } } viewBox="0 0 24 24" width="1em" xmlns="http://www.w3.org/2000/svg" > <circle cx="12" cy="12" r="10" /> <line x1="12" x2="12" y1="8" y2="12" /> <line x1="12" x2="12.01" y1="16" y2="16" /> </svg>

src attr:
data:image/svg+xml;utf8,%3Csvg%20fill%3D%22none%22%20height%3D%221em%22%20stroke%3D%22currentColor%22%20strokeLinecap%3D%22round%22%20strokeLinejoin%3D%22round%22%20strokeWidth%3D%222%22%20style%3D%7B%20Object%20%7B%20%22color%22%3A%20undefined%2C%20%7D%20%7D%20viewBox%3D%220%200%2024%2024%22%20width%3D%221em%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20%3E%20%3Ccircle%20cx%3D%2212%22%20cy%3D%2212%22%20r%3D%2210%22%20%2F%3E%20%3Cline%20x1%3D%2212%22%20x2%3D%2212%22%20y1%3D%228%22%20y2%3D%2212%22%20%2F%3E%20%3Cline%20x1%3D%2212%22%20x2%3D%2212.01%22%20y1%3D%2216%22%20y2%3D%2216%22%20%2F%3E%20%3C%2Fsvg%3E

from sanity-plugin-icon-picker.

@blueputty01 Thank you for digging deeper into this. Your proposal of removing the style object before passing it to pretty-format seems like the most clean solution to me.

How ever I just discovered another problem with the parsing of pretty-format which cause inconsistencies in the original SVG (from react icons) and the one saved to later be used in the client.

Issue:

Using fi alert circle

Left icon is the parsed svg string after running it through pretty-format, right icon is the original one from react-icons.

Comparison:

Left (parsed)

<svg fill="none" height="1em" stroke="currentColor" strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" viewBox="0 0 24 24" width="1em" xmlns="http://www.w3.org/2000/svg"><circle cx="12" cy="12" r="10" /><line x1="12" x2="12" y1="8" y2="12" /><line x1="12" x2="12.01" y1="16" y2="16" /></svg>

Right (original)

<svg stroke="currentColor" fill="none" stroke-width="2" viewBox="0 0 24 24" stroke-linecap="round" stroke-linejoin="round" style="width: 1.5em; height: 1em;" height="1em" width="1em" xmlns="http://www.w3.org/2000/svg"><circle cx="12" cy="12" r="10"></circle><line x1="12" y1="8" x2="12" y2="12"></line><line x1="12" y1="16" x2="12.01" y2="16"></line></svg>

The difference in attribute order when comparing the two made it quite difficult to see the issue, but the issue is the attribute casing for some attributes where it's using kebab casing for the original one (as it should) but using camel casing for the parsed one. As an example strokeLinecap for parsed and stroke-linecap for original.

One possible solution might be to simply convert all occurrences of camel casing to kebab casing but this might also create more problems since, for instance, attribute viewBox is using camel casing in both cases. I will have to do some research on this.

from sanity-plugin-icon-picker.

@blueputty01 Created a draft PR for this. I think this should work. Please let me know what you think and if you see any potential improvements. Not really found of having to define an array of ignores for the camel cased attributes and unaware if all of them are really required for this use case #39

from sanity-plugin-icon-picker.

@christopherafbjur I'll be honest, I don't know much about these testing libraries.

However, @testing-library/react seems to avoid all the issues we identified in the generated code (casing, stray objects) with a potentially cleaner implementation:

import { render } from '@testing-library/react';
...
const svgString = render(icon.component).baseElement;

Generated fi alert circle (diff seems to be an additional style object with height and width):
<svg stroke="currentColor" fill="none" stroke-width="2" viewBox="0 0 24 24" stroke-linecap="round" stroke-linejoin="round" height="1em" width="1em" xmlns="http://www.w3.org/2000/svg"><circle cx="12" cy="12" r="10"></circle><line x1="12" y1="8" x2="12" y2="12"></line><line x1="12" y1="16" x2="12.01" y2="16"></line></svg>

from sanity-plugin-icon-picker.

@christopherafbjur Oops! That's definitely not ideal.

Took a deeper look into your PR, and I don't see any direct improvements I could recommend!

Alternatively, what if we took advantage of the existing element from the sanity UI button? This would resolve our casing issue.

Eg:

This would obviously need additional typings, but I'd be happy to open a PR to add that myself if you'd like.

IconPicker:

  const setIcon: SearchResultsOnSelectCallback = (
    icon: IconObject,
    ele: HTMLButtonElement
  ) => {
    if (selected && icon.name === selected.name) return unsetIcon();

    const svgString = ele.getElementsByTagName('svg')[0].outerHTML; // <--here

SearchResults:

  function IconButton(icon: IconObject) {
    const buttonRef = useRef<HTMLButtonElement>(null);

    return (
      <Button
        ref={buttonRef} // <--here
        key={icon.provider.concat(icon.name)}
        mode="ghost"
        onClick={() => onSelect(icon, buttonRef.current)}
        text={<icon.component />}
        style={{ marginTop: '5px' }}
        selected={
          !!selected &&
          selected.provider === icon.provider &&
          icon.name === selected.name
        }
      />
    );
  }

from sanity-plugin-icon-picker.

@blueputty01 That looks elegant! I haven't tested this but by the looks of it, it should do the trick and should solve the attribute casing things with quite a reduced amount of lines. Feel free to create a PR 🙂

from sanity-plugin-icon-picker.

any update on it?

i really need it to reduce my frontend package.

maybe i can make the PR if it can help

from sanity-plugin-icon-picker.

@JJK801 Thanks for reminding me! The past week was really hectic. I'll put in a PR today :)

from sanity-plugin-icon-picker.