Example for getSVG not working

[schlomo]

This example doesn't work:

const { SVG, Collection } = require('@iconify/json-tools');

let collection = new Collection();
collection.loadIconifyCollection('mdi');
let svg = new SVG(collection.getIconData('home'));
console.log(svg.getSVG());

It fails like this:

$ node c.js
/Users/schlomoschapiro/src/iconify-basic-bug/node_modules/@iconify/json-tools/src/svg.js:415
        Object.keys(props).forEach(name => {
               ^

TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at Function.splitAttributes (/Users/schlomoschapiro/src/iconify-basic-bug/node_modules/@iconify/json-tools/src/svg.js:415:10)
    at SVG.getSVG (/Users/schlomoschapiro/src/iconify-basic-bug/node_modules/@iconify/json-tools/src/svg.js:369:24)
    at Object.<anonymous> (/Users/schlomoschapiro/src/iconify-basic-bug/c.js:6:17)
    at Module._compile (internal/modules/cjs/loader.js:1200:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1220:10)
    at Module.load (internal/modules/cjs/loader.js:1049:32)
    at Function.Module._load (internal/modules/cjs/loader.js:937:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
    at internal/main/run_main_module.js:17:47

The reason is that the SVG.getSVG() function expects the data as an argument instead of accessing the instance variable _item.

When I change svg.js like this, then it works correctly:

getSVG(addExtra) {
    const props = this._item;

Is there another way to easily retrieve the SVG definitions for a bunch of icons in Node.js?

[cyberalien]

Parameter to getSVG() is not optional, but examples show that it is.

Change last line to:

console.log(svg.getSVG({}));

Thanks for noticing issue in readme! Fixed.

[schlomo]

Sorry, I disagree. IMHO the problem is that the constructor new SVG(data) initializes this._item but the getSVG() function doesn't make use of that variable at all.

I think that changing the getSVG() function like this would be the correct fix. It also fixes the wrong handling of the addExtra optional parameter:

getSVG(addExtra) {
        var props = this._item;
    let attributes = SVG.splitAttributes(props),
        data = this.getAttributes(attributes.icon);

    let svg =
        '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"';

    // Add extra attributes - assume that their names and values are escaped
    if (addExtra) {
        Object.keys(addExtra).forEach(attr => {
            svg += ' ' + attr + '="' + addExtra[attr] + '"';
        });
    }

    // Add SVG attributes
    Object.keys(data.attributes).forEach(attr => {
        svg += ' ' + attr + '="' + data.attributes[attr] + '"';
    });

    // Add style with 360deg transformation to style to prevent subpixel rendering bug
    svg +=
        ' style="-ms-transform: rotate(360deg); -webkit-transform: rotate(360deg); transform: rotate(360deg);';
    Object.keys(data.style).forEach(attr => {
        svg += ' ' + attr + ': ' + data.style[attr] + ';';
    });
    if (props && props.style !== void 0) {
        svg += props.style;
    }
    svg += '">';

    svg += data.body + '</svg>';

    return svg;
}

[cyberalien]

You are confusing 2 different variables.

this._item represents icon data, such as width, height, body.

Parameter props that was missing in example represents customisations that should be applied to icon before rendering it, such as changing width/height, rotating icon, flipping icon and any extra attributes that should be passed to svg element.

In getSVG value of this._item is used, but it is used in getAttributes call that combines icon data (from this._item) and customisations (from props parameter) to generate data that getSVG turns into svg tag.

[schlomo]

Ah, I see. I re-read the code and get a better understanding. Thank you. Can you please also add an illustrative example for using the addExtra parameter?

[cyberalien]

That's for extra attributes that are not part of code generated by default.

For example:

console.log(
  svg.getSVG({
    height: "2em",
    title: "Test",
  })
);

In this case height will be changed to "2em", which will also change icon width. However title is not part of icon customisations, so it will be ignored.

If you set second parameter to true, all custom attributes, such as title in example above, will be added to generated icon.

console.log(
  svg.getSVG({
    height: "2em",
    title: "Test",
  }, true)
);

[schlomo]

Got it, thanks. Might be useful to add that as well to the README.md

[schlomo]

FYI, https://gist.github.com/schlomo/0d39fbaabb45861b7586757f3822e4d4 is an example of what I was trying to do. Just for fun and to better understand how all of this works.

Good news is that indeed SVG sprites have no performance benefit, but using Iconify.getSVG() has a major performance boost compared to adding <iconify-icon> elements. Probably not really a surprise.

The end result is now in https://github.com/schlomo/chromebooks-in-deutschland/commit/f53a68721be433258a9977e96ed012ee2841dd42

[cyberalien]

Yes, it is much faster.

If you use <iconify-icon /> or <span class="iconify" />, browser first adds that element, then dispatches observer event, then Iconify finds those elements, checks if they exist, gets data using getSVG function, then creates span element, renders SVG inside it (it had to be done because Internet Explorer doesn't support innerHTML on SVG and messes up case sensitive attributes), then swaps placeholder and rendered SVG. It is much longer process than simply rendering output of getSVG() function.