search

phosphorjs / phosphor

The PhosphorJS Library
BSD 3-Clause "New" or "Revised" License
1.04k stars 169 forks source link

Datagrid: Asynchronous TextRenderer functions #403

Closed martinRenou closed 5 years ago

martinRenou commented 5 years ago

We would like our users to provide their own TextRenderer.backgroundColor functions. But we are not sure that their code is not malicious so we run it using a WebWorker. This means that our functions are asynchronous, but due to the current implementation of the resolveOption function in the phosphor/packages/datagrid/src/cellrenderer.ts file, it does not seem to be possible:

  export
  function resolveOption<T>(option: ConfigOption<T>, config: ICellConfig): T {
    return typeof option === 'function' ? option(config) : option;
  }

Should I open a PR for allowing asynchronous functions?

sccolbert commented 5 years ago

Hmmm, you really want these functions to be synchronous for best performance. Anything asynchronous on the rendering path is going make the grid dog slow and consume a bunch of memory.

sccolbert commented 5 years ago

How is the user providing the code to be run?

sccolbert commented 5 years ago

Not to mention the cost of serializing/deserializing the data values to cross the thread boundary....

sccolbert commented 5 years ago

What's your actual threat model here?

martinRenou commented 5 years ago

I totally agree. I'll try to make my functions synchronous and not use web workers (Going through web workers and the messaging logic would have slown down the rendering even more). Feel free to close this issue.

martinRenou commented 5 years ago

Just to give more information on our use case.

We are exposing the PhosphorJS grid to Python in Jupyter. We want our users to be able to provide conditional formatting from Python. For this purpose, they either need to type JavaScript code (and then we need to run this code safely, e.g. using Web Workers), or we can find a way around. Our idea would be to expose to Python built-in tweakable formatters.