Fix `.indexOf()` call on integers when using numbers as node id's

finn-vgtl commented 2 years ago

First of all, I'd like to say thank you for this amazing library! Its scope is simply perfect for my use-case.

While implementing redot in my project I've used the following dot notation for testing purposes. (Notice how I used integers for my node id's)

digraph {
  1 [label = A]
  2 [label = B]
  3 [label = C]
  1 -> 2
  2 -> 3
}

Converting this digraph to an AST using redot.parse() worked absolutely perfectly, however, using redot.stringify() to convert the newly generated AST back to a DOT-notation threw an error:

[…]/node_modules/redot-stringify/redot-stringify.js:5
  if (text.indexOf(" ") > 0) {
           ^

TypeError: text.indexOf is not a function
    at utilQuotesAndEscaping ([…]/node_modules/redot-stringify/redot-stringify.js:5:12)
    at nodeId ([…]/node_modules/redot-stringify/redot-stringify.js:58:10)
    at Array.map (<anonymous>)
    at utilProcessChildType ([…]/node_modules/redot-stringify/redot-stringify.js:24:6)
    at nodeStatement ([…]/node_modules/redot-stringify/redot-stringify.js:41:5)
    at Array.map (<anonymous>)
    at utilProcessChildType ([…]/node_modules/redot-stringify/redot-stringify.js:24:6)
    at graph ([…]/node_modules/redot-stringify/redot-stringify.js:94:5)
    at digraph ([…]/node_modules/redot-stringify/redot-stringify.js:114:10)
    at Array.map (<anonymous>)

Node.js v18.2.0

After a bit of debugging, I found out that using strings as node id's won't trigger the error so I've implemented an extra step in the .utilQuotesAndEscaping() function to ensure compatibility with integers in my local setup. Obviously, this is a temporary solution so I'm proposing these changes directly to you:

TL:DR

This PR proposes a small change to convert the text parameter of the function .utilQuotesAndEscaping() to string before trying to call .indexOf() on it, ensuring the latter function is available in the prototype, thus preventing errors.

Thanks for considering my PR! 🙋‍♂️

Drincann commented 1 year ago

When will this patch be released to the npm repository?

ChristianMurphy commented 1 year ago

It will be released when the next major is ready, that is pending finalization in https://github.com/redotjs/redot/pull/25 Pull requests to help move the next major forward are welcome. Otherwise be patient.

redotjs / redot

Fix `.indexOf()` call on integers when using numbers as node id's #23

TL:DR