implement http content-negotiation for plotly-graph#228
Conversation
test/integration/orca_serve_test.js
Outdated
| }) | ||
| }) | ||
|
|
||
| t.test('it should override format provided in payload', t => { |
There was a problem hiding this comment.
I'm not sure about this one... 🤔
Problematic case: the user asks for SVG in HTTP header but also ask for PNG in the payload via format attribute. Which one should win?
There was a problem hiding this comment.
I think you have it the right way here: header overriding payload.
src/app/server/create-server.js
Outdated
|
|
||
| pending++ | ||
| comp._module.parse(body, compOpts, sendToRenderer) | ||
| comp._module.parse(body, compOpts, sendToRenderer, req) |
There was a problem hiding this comment.
Hmm. I would prefer leaving sendToRenderer as the last argument, as callbacks sendToRenderer are usually passed last.
I think having
comp._module.parse(body, req, compOpts, sendToRenderer)would be best ... and don't forget to update:
https://github.com/plotly/orca/blob/master/CONTRIBUTING.md#nomenclature-for-ipc-callbacks
accordingly.
| const path = require('path') | ||
| const { spawn } = require('child_process') | ||
| const electronPath = require('electron') | ||
| const electronPath = process.env['ELECTRON_PATH'] ? process.env['ELECTRON_PATH'] : require('electron') |
There was a problem hiding this comment.
It is not mandatory for this PR but it provides a way to use a manually installed electron in the event that electron-download fails to install pre-built binary for your platform.
I could remove it but it's a must-have in my current dev environment and maybe it is also the case for others. It simply gives more flexibility. What do you think?
| }) | ||
| }) | ||
|
|
||
| t.test('it should do HTTP content-negotation for *plotly-graph* component', t => { |
There was a problem hiding this comment.
What happened to your other content-negotiation tests? Don't you want to test a valid content negotiation?
|
Nicely done 💃 |
2 participants
Closes #226
Check added tests for details!
accept: */*andaccept: *