Skip to content
This repository has been archived by the owner on Feb 12, 2019. It is now read-only.

svgson.stringify should serialize object or array types for data attribute #8

Closed
ceremcem opened this issue Oct 17, 2018 · 10 comments
Closed

Comments

@ceremcem
Copy link
Contributor

When we need to stringify complex data types (such as Object or Array) in data- attributes, those values should be stringified before converting to SVG.

Alternatively, svgson.stringify might accept a transformNode like option where we could manually convert any data format with a custom function.

@elrumordelaluz
Copy link
Owner

Could you please give an example of complex data types in the svgson Object you need to stringify? not sure I catch what you mean 🙏

@ceremcem
Copy link
Contributor Author

Here is the SVG content:

<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="602" height="402" viewBox="0 0 602 402">
  <g fill="none" fill-rule="nonzero" stroke="none" stroke-width="1" stroke-linecap="butt" stroke-linejoin="miter" stroke-miterlimit="10" stroke-dasharray="" stroke-dashoffset="0" font-family="sans-serif" font-weight="normal" font-size="12" text-anchor="start" style="mix-blend-mode: normal" />
  <g fill="#000000" fill-rule="nonzero" stroke="none" stroke-width="1" stroke-linecap="butt" stroke-linejoin="miter" stroke-miterlimit="10" stroke-dasharray="" stroke-dashoffset="0" font-family="none" font-weight="none" font-size="none" text-anchor="none" style="mix-blend-mode: normal">
    <path d="M37.79528,78.08504v-40.28976h31.67244v40.28976z" data-paper-data="{&amp;quot;project&amp;quot;:{&amp;quot;layer&amp;quot;:&amp;quot;scripting&amp;quot;}}" />
    <path d="M86.81575,60.25516v-3.84h11.52756v3.84z" data-paper-data="{&amp;quot;project&amp;quot;:{&amp;quot;layer&amp;quot;:&amp;quot;scripting&amp;quot;}}" />
    <path d="M86.81575,53.8224v-3.84h11.52756v3.84z" data-paper-data="{&amp;quot;project&amp;quot;:{&amp;quot;layer&amp;quot;:&amp;quot;scripting&amp;quot;}}" />
    <path d="M86.81575,47.38965v-3.84h11.52756v3.84z" data-paper-data="{&amp;quot;project&amp;quot;:{&amp;quot;layer&amp;quot;:&amp;quot;scripting&amp;quot;}}" />
    <path d="M86.81575,66.68791v-3.84h11.52756v3.84z" data-paper-data="{&amp;quot;project&amp;quot;:{&amp;quot;layer&amp;quot;:&amp;quot;scripting&amp;quot;}}" />
    <path d="M86.81575,73.12067v-3.84h11.52756v3.84z" data-paper-data="{&amp;quot;project&amp;quot;:{&amp;quot;layer&amp;quot;:&amp;quot;scripting&amp;quot;}}" />
  </g>
  <g fill="none" fill-rule="nonzero" stroke="none" stroke-width="1" stroke-linecap="butt" stroke-linejoin="miter" stroke-miterlimit="10" stroke-dasharray="" stroke-dashoffset="0" font-family="sans-serif" font-weight="normal" font-size="12" text-anchor="start" style="mix-blend-mode: normal" />
</svg>

You can see there is a JSON.stringifyied and htmlEncoded data in data-paper-data= attribute. Those values are actually objects.

svgson can not know what to do with those objects, so we should properly convert them into objects while svgson.parseing:

transformNode = (node) ->
    if node.attributes["data-paper-data"]
        node.attributes["data-paper-data"] = JSON.parse htmlDecode that
    node

json <~ svgson.parse _svg, {transformNode} .then
console.log JSON.stringify json, null, 2

...which produces JSON output correctly (unrelated parts are removed for clarity)

{
  "name": "svg",
  ...
  "children": [
    {
      ...
      "children": [
        {
          "name": "path",
          "type": "element",
          "value": "",
          "attributes": {
            "d": "M37.79528,78.08504v-40.28976h31.67244v40.28976z",
            "data-paper-data": {
              "project": {
                "layer": "scripting"
              }
            }
          },
          ...
    }
  ]
}

Now we can manipulate the AST as we wish, add/remove some properties, and re-generate SVG:

svg = svgson.stringify json

Here we end up with buggy SVG: (unrelated parts are removed for clarity)

...
    <path d="M37.79528,78.08504v-40.28976h31.67244v40.28976z" data-paper-data="[object Object]" />
    <path d="M86.81575,60.25516v-3.84h11.52756v3.84z" data-paper-data="[object Object]" />
...

Here you can see the complex data types are not stringified correctly.

Current workaround

Currently I'm stringifying those data: attributes before passing the JSON AST to svgson.stringify().

@elrumordelaluz
Copy link
Owner

Thanks @ceremcem now I get it! In fact the parser is not responsible for what is inside each attr/prop, and as you shown you could achieve this using transformNode.

If I undertsand your current workaround is that you add a middle step looping nodes to JSON.stringyify (and htmlEncod) those data-* you need.

svgson.stringify should serialize object or array types for data attribute

Do you think is responsability of svgson.stringify to loop, check and convert those values?

Same situation in the parser that could be also responsible to parse values that contains stringified Objects and so on. What do you mean. My first thougt is that is specific on uses cases.

Interested in analyse if could be a feature or some thing to explain in docs.

@ceremcem
Copy link
Contributor Author

Do you think is responsability of svgson.stringify to loop, check and convert those values?

On second thought, as we both agree that parsing a data-foo= attribute is not a responsibility of svgson.parse, this will also mean that stringifying an object shouldn't be a responsibility of svgson.stringify. If it won't parse, it shouldn't stringify.

However, no data should be eaten up by svgson.stringify. Which means, it should throw a proper error that indicates the data needs to be String or Number. Then we would stringify those values on our own.

If I undertsand your current workaround is that you add a middle step looping nodes to JSON.stringyify (and htmlEncod) those data-* you need.

Yes, and it needs to be recursive. Although it's not hard to implement such a middle step, it reduces readability of resulting code. So my proposal boils down to those 2 features:

  • Proper error: svgson.stringify must throw an error for values under attributes that holds any value different from String or Number
  • transformNode function: svgson.stringify should accept such a function, just like svgson.parse supports.

@elrumordelaluz
Copy link
Owner

sound good your both proposals for stringify so I am starting to think that instead of do a simple conversion form parsed into stringified, that method have also to check if the AST passed fits or have keys wrong named etc.

That means, make a little tweaks on stringify method to make if 100% symmetrical (parser ~ stringify)

@elrumordelaluz
Copy link
Owner

You could try in svgson-next@4.3.0.

In tests there is an example with a similar situation you described above.

Let me know,

@ceremcem
Copy link
Contributor Author

FYI: I still couldn't have a time to test this, but it's on my mind.

@elrumordelaluz
Copy link
Owner

no problem @ceremcem, let me know if there are issues, keep in mind that everything was moved into svgson@3.x so you could also update your deps.

@ceremcem
Copy link
Contributor Author

FYI: Nicely works! Thanks!

@elrumordelaluz
Copy link
Owner

Awesome, thanks for the feedback

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants