remove new Function calls

[firien]

new Function may violate some sites Content Security Policy which can disable evals. All tests are passing, but I don't know if you want to replicate the conditional existence of these functions and throw errors. For instance throw an error if relativeToGetter is called and @options.relativeTo is not defined.

I'm also not sure what is allowed as @type for VersionedStruct - is it always a string representing a single property? or can it be a chain like @options.relativeTo on Pointer

[coveralls]

Coverage decreased (-0.2%) to 98.682% when pulling fe54419 on firien:remove-eval into 74c29e3 on devongovett:master.

[coveralls]

Coverage decreased (-0.4%) to 98.499% when pulling 76dbc93 on firien:remove-eval into 74c29e3 on devongovett:master.

[firien]

@devongovett did you get a chance to look at this?
The code coverage decrease is because these were conditional functions. These conditional functions may have been created (via new Function), but i think this reveals they were never executed afterwards.

[coveralls]

Coverage decreased (-0.2%) to 98.689% when pulling 22910ed on firien:remove-eval into 74c29e3 on devongovett:master.

[Brvtvs]

This change would be very helpful so that this library can be used with a secure CSP.

[jfoclpf]

Apologies cause I'm still a bit newbie on these github issues.

Considering that "this branch has no conflicts with the base branch", does it mean that the issue is already solved or immediately solvable? Thank you so much in advance

[firien]

"no conflicts" just means that the branch can be merged without a user having to manually fix conflicts. It does not guarantee integrity of the actual code - it could very well contain a syntax error…

It is the testing that matters, and that is passing; but the code coverage has some complaints…

@devongovett, would you have any time to look into this?

[jfoclpf]

Thanks for the clarification.

…

On Thu, 3 May 2018, 14:55 firien, ***@***.***> wrote: "no conflicts" just means that the branch can be merged without a user having to manually fix conflicts. It does not guarantee integrity of the actual code - it could very well contain a syntax error… It is the testing that matters, and that is passing; but the code coverage has some complaints… @devongovett <https://github.com/devongovett>, would you have anytime to look into this? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#22 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADzODUhuTq74RzGNHV-L0nAzJOamT65Tks5tuv5RgaJpZM4LeTSQ> .

[clintonium-119]

@firien I'm running your fix in a local build of devongovett/pdfkit, and it has removed the unsafe-eval warnings, but Its taking many (guessing at least 10) times longer to generate the pdf.

Did you notice any significant performance degradation in your usage?

[firien]

I actually don't use it in production; I still use an older pdfmake that doesn't have evals.

But, I just wired it up and didn't notice any difference in speed. Although I am generating very small PDFs, both versions were taking ~300ms.

---

I can see how making an Array and reducing it may generate some overhead. @devongovett would know better than I if this function is cache-able.

[clintonium-119]

Thanks for checking. I believe it to be a problem with my local build process.

[jarrettj]

+1

[ArthurClemens]

fontkit tests fail with the changes in VersionedStruct.

Updated version in PR #29

[firien]

From initial PR:

I'm also not sure what is allowed as @type for VersionedStruct - is it always a string representing a single property? or can it be a chain like @options.relativeTo on Pointer

According to @ArthurClemens, the answer is: it can be a property chain.

There should probably be some local tests for this.

[firien]

updated for es6

[firien]

@devongovett, this PR has been updated for ES6, can you please review it? Any feedback would be appreciated.
Thank you.

[devongovett]

Closing in favor of #34. This is an API change though, so it'll be a major version bump.