search

d3 / d3-dsv

A parser and formatter for delimiter-separated values, such as CSV and TSV.
https://d3js.org/d3-dsv
ISC License
437 stars 76 forks source link

Refactor to avoid function constructor in the form of eval #80

Closed archmoj closed 3 years ago

archmoj commented 3 years ago

Seems like this refactor works to avoid security warnings/issues. Also it would be nice to have a similar patch for d3 v3. see: https://github.com/archmoj/d3/commit/93e74e544e9332ecbd17b43b3694de511310be6e

cc: @curran @tdelmas https://github.com/d3/d3-dsv/pull/67 https://github.com/d3/d3-dsv/pull/65 cc: @alexcjohnson @nicolaskruchten https://github.com/plotly/plotly.js/issues/897

mbostock commented 3 years ago

Please read https://github.com/d3/d3-dsv/pull/26#issuecomment-298638569.

archmoj commented 3 years ago

Thanks for the note. The attempt in #26 wasn't as fast while using reduce. I don't really see a notable performance difference between parsing a 1024x1024 table using current (master) and (this) new implementation. So please consider reopening. Cheers.

tdelmas commented 3 years ago

@archmoj Could you provide your results with the latest Chrome and Firefox in the same format that https://github.com/d3/d3-dsv/pull/26#issuecomment-298660209 ?

mbostock commented 3 years ago

If the substantial performance penalty of doing this “safely” (scare quotes because it’s already safe, just not in a way that a browser can statically analyze and know ahead of time) I’m happy to reconsider. I’m also curious if Object.entries, possibly with a generator rather than a materialized array, is faster than assigning to an object. I doubt it, but sometimes browsers have clever optimizations.

tyn1998 commented 2 years ago

In my case, I'm developing a chrome extension under manifest v3, which refuses CSP with 'unsafe-eval'. So this PR helps! Thank you!