TypeError: t.toFixed is not a function

[ianchanning]

Describe the bug Using strings for height and width in the options object generates an error:

                responseDoc
                    .svg(svgRef.current, {
                        height: '18',
                        width: '80',
                    })

Uncaught (in promise) TypeError: t.toFixed is not a function
    roundToPrecision jspdf.es.min.js:86
    hpf jspdf.es.min.js:86
    rect jspdf.es.min.js:86
    render svg2pdf.es.min.js:41
    ...

Now that I know the problem the error is obvious (toFixed on a string throws an error), but I actually thought it was a problem with my SVG React element, so went through every possible combination of trying to get that working before considering that something so simple as my options were wrong.

What version are you using (exact version of svg2pdf.js and jspdf - the yWorks fork version, of course)? jspdf: 2.5.1 svg2pdf.js: v2.2.1

To Reproduce Steps to reproduce the behavior:

1. With the following SVG and JavaScript code

import React, { useRef } from 'react'
import { jsPDF } from 'jspdf'
import 'svg2pdf.js'

const HelloPdf = () => {
  const certificateTemplateRef = useRef(null)
  const svgRef = useRef(null)
  const generatePdf = () => {
    const doc = new jsPDF({ format: 'a4', unit: 'px' })
    doc.html(certificateTemplateRef.current, {
      async callback(responseDoc) {
        // save the document as a PDF with name of Memes
        responseDoc
          .svg(svgRef.current, {
            height: '18',
            width: '80'
          })
          .then(() => {
            responseDoc.save('svgPDF')
          })
      }
    })
  }
  return (
    <div
      style={{
        display: 'flex',
        alignItems: 'center',
        flexDirection: 'column'
      }}
    >
      <button type="button" onClick={generatePdf}>
        PDF
      </button>
      <div ref={certificateTemplateRef}>
        <div>
          <p>Some text</p>
        </div>
      </div>
      <div>
        <svg
          xmlns="http://www.w3.org/2000/svg"
          viewBox="0 0 300 150"
          ref={svgRef}
        >
          <text x="20" y="20">
            Hello, world!
          </text>
        </svg>
      </div>
    </div>
  )
}
export default HelloPdf

3. I get this result
4. Or see this error

Uncaught (in promise) TypeError: t.toFixed is not a function
    roundToPrecision jspdf.es.min.js:86
    hpf jspdf.es.min.js:86
    rect jspdf.es.min.js:86
    render svg2pdf.es.min.js:41
    ...

More specifically that breaks here in the minified code:

...throw new Error("Invalid argument passed to jsPDF.roundToPrecision");return t.toFixed(n).replace(/0+$/,"")...

So it's actually an error that is thrown from jsPDF: https://github.com/parallax/jsPDF/blob/2d9a91916471f1fbe465dbcdc05db0cf22d720ec/src/jspdf.js#L496

Expected behavior I would have liked it to just work, I'm using Javascript which is usually forgiving if you use the wrong type. I did look at the types.d.ts, but I wasn't really considering that my options were of the wrong type.

Given that the number type of the options is strict, could you perhaps include in the README documentation that the options must be numbers. Currently you just have:

doc
  .svg(element, {
    x,
    y,
    width,
    height
  })

which gives no indication that the types are strict.

Adding something like the regular jsPDF documentation of doc.html would be very helpful:

svg(element, options) → Promise<jsPDF>

Example

...

Parameters

Name	Type	Attributes	Description
element	HTMLElement		The source svg HTMLElement.
options	Object	\<optional>	Name Type Attributes Description x number <optional> The horizontal offset at which the SVG shall be rendered. The default is 0. y number <optional> The vertical offset at which the SVG shall be rendered. The default is 0. width number <optional> The desired width of the rendered SVG. Defines the initial viewport for the outermost SVG element. The width and height properties behave exactly like the width and height attributes on an HTML img element with an SVG image as source. height number <optional> The desired height of the rendered SVG. See width. loadExternalStyleSheets boolean <optional> Whether external style sheets referenced by SVG link elements or xml-stylesheets shall be loaded using HTTP requests. Note, that all style sheets that cannot be accessed because of CORS policies are ignored. The default is false.

Screenshots N/A

Desktop (please complete the following information):

• OS: Linux (Pop OS - Ubuntu derivative)
• Browser: Firefox
• Version: 103.0

Smartphone (please complete the following information): N/A

Additional context Some react examples for your code would be really useful. All the examples I found rely on document.getElementById or some combination of parsing SVG text and then picking sometimes the first child and sometimes not.

If it's of any use I got my original jsPDF code from here: https://codesandbox.io/s/generate-pdf-using-jspdf-kom01x?file=/src/generatePdf/GeneratePdf.tsx

I then wanted to add a SVG to it.

[yGuy]

Sorry, but this is still a programming library and it requires at least some basic programming skills to use it. For those who consider numbers and strings to be equal, they probably should not be using it. At least I would not want to optimize for these users.

JavaScript is not 'forgiving if you use the wrong type' - it will typically just do the wrong thing. So in a way, getting an exception is already an improvement, because if we automatically converted strings to numbers, the next programmer would claim that it doesn't work if they to height: smallSize+largeSize when smallSize = "1337" and largeSize = "42" and the result is not 1379 but 133742. That would not be a problem of the library, either, but it's simply a bug in the code of the user.

I agree that we could check the arguments more thoroughly to create nicer exceptions, and I am happy to accept a pull-request for thorough runtime parameter checks, but I do not support the proposed change:

Secretly doing this kind of auto conversion doesn't prevent bugs but actually hides them, IMHO.

[ianchanning]

I think you mis-read my request - I'd hoped it would 'just work' but I wasn't proposing that you fix that or do some auto-conversion.

This was my request:

Given that the number type of the options is strict, could you perhaps include in the README documentation that the options must be numbers.

...

Adding something like the regular jsPDF documentation of doc.html would be very helpful

I will see if I can create a PR for the documentation

[yGuy]

OK, understood.

I would argue that for the purpose of documenting the API (and actually verifying your correct use) you should be using the typescript typings, which indeed document this requirement, clearly:

https://github.com/yWorks/svg2pdf.js/blob/6b5b4ea98aa2e5a4e2d2c7c8c2937da8cfdadafb/src/svg2pdf.ts#L66

If you were using Typescript, the compiler would have found the problem and told you about the correct usage.

[ianchanning]

Understood, clearly you and I have very different ideas about documentation and who are the type of people who should be using your library. I am clearly not one of the people you want using your library so I will leave you to develop it as you see fit.

Thank you for your library.

[yGuy]

But the Readme already says that you should inspect the linked typings file. And that file is actually really short and does contain the information you need.https://github.com/yWorks/svg2pdf.js/blob/6b5b4ea98aa2e5a4e2d2c7c8c2937da8cfdadafb/types.d.ts#L43I think the Readme itself is quite long,  already.I totally agree with you that having a page like the one for jspdf would be a great improvement.I personally find it very obvious that an API that is working with scalar values in JS should be using numbers and not strings. As such adding this information explicitly to the Readme could even hurt because if we start adding stuff like this no one will be reading the file because of its sheer size. Like the Apple EULA 😉Let's create a new ticket specifically for creating and linking to a nicer API documentation page. 

[yGuy]

Don't know why the last comment appears to have been added 34 minutes ago, only. I wrote and sent that email before on the second or 3rd of September, more than a month ago. Did you see that response before? Strange...