search

fivethirtyeight / d3-pre

Pre-render d3 visualizations
MIT License
608 stars 20 forks source link

Fails when displaying page in actual Electron window #5

Open rahatarmanahmed opened 7 years ago

rahatarmanahmed commented 7 years ago

Yoooooo, I spent like 3 hours digging through browserified & minified fivethirtyeight production code trying to figure out why the 2016 forecast page didn't render graphs when I embedded it in an Electron app. (Trying to make a wall display of the election forecast using y'alls visualizations).

Eventually I figured out it was because of this line: https://github.com/fivethirtyeight/d3-pre/blob/master/src/index.js#L17 which recognizes electron even if I disable nodeIntegration or isolate the page in a webview tag

Easy fix for me, I can just change the userAgent. But I thought I'd let you know for the miniscule chance that it might be useful for you. I would suggest using something like is-electron-renderer. Feel free to close this issue if you don't care, though.

mathisonian commented 7 years ago

Hmm that is an interesting use case, thanks for pointing it out. Maybe I'm wrong but wouldn't you would still be facing a similar problem if we used that other module?

Maybe its better to set some kind of explicit flag in d3-pre-renderer and check for that here.

rahatarmanahmed commented 7 years ago

Oh yeah is-electron-renderer wouldn't exactly work, my bad. But you can see how they detect a node-like environment by checking for the existence of process. You just need to check for that instead of the userAgent (which will always say Electron even if you disable node integration).