Skip to content
  • Sponsor plotly/react-plotly.js

  • Notifications You must be signed in to change notification settings
  • Fork 137

Hook-based API #275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -113,6 +113,34 @@ In short, this means that simply adding data points to a trace in `data` or chan

## API Reference

### usePlotly Hook

As an alternative to the `Plot` component, you may use the `usePlotly` react _hook_. This provides a more powerful API with full control over the plot element, compatibility with functional components, intuitive responsive behaviour and ability to use `extendTraces`.
Here is a simple example of creating a chart with `usePlotly`:

```jsx
function MyChart(props) {
const { ref, updates, appendData } = usePlotly();

// Here is a function that will change the data. You must pass a partial Figure object (plotly DSL object) which will be
// merged with all previous calls to `updates`
const changeData = () => updates({ data: [ { y: [Math.random() * 10], type: 'scatter' } ] })

// Here we start extending traces using the `appendData` stream
const extendData = setInterval(() => {
appendData({ data: { y: [[Math.random() * 10]]}, tracePos: [0] });
}, 500);

return (
<div>
<div ref={ref} style={{ width: '500px', height: '300px' }}/>
<button onClick={changeData}>React!</button>
<button onClick={extendData}>Extend!</button>
</div>);
}
```


### Basic Props

**Warning**: for the time being, this component may _mutate_ its `layout` and `data` props in response to user input, going against React rules. This behaviour will change in the near future once https://github.com/plotly/plotly.js/issues/2389 is completed.
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -70,7 +70,9 @@
},
"peerDependencies": {
"plotly.js": ">1.34.0",
"react": ">0.13.0"
"react": ">0.13.0",
"flyd": ">=0.2.8",
"ramda": ">=0.28.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flyd and ramda seem like they'd be better as dependencies rather than peerDependencies - and then let's change from >= to ^

Copy link

@floriancargoet floriancargoet Oct 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are 2 news deps really required?
ramda seems to be used just to write this code in the functional style.
The author himself said in the related issue that it could be replaced with:

function getSizeForLayout(entries) {
    const { width, height } = entries[0].contentRect;
    return { layout: { width, height } };
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question @floriancargoet - ramda has a couple of other uses in this PR (and we do use it a good deal elsewhere at Plotly), but to my mind your getSizeForLayout rewrite is more readable than the full functional version.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While yes getSizeForLayout can be written easily without ramda (happy to do this), the main reason it was included was for mergeDeepRight which is a relatively complex function to implement.
We could go for an independent implementation but my argument would be that ramda is relatively small and compatible with tree shaking

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, deep merge is annoying to reimplement. I have no problem adding this (and flyd) but I do think they belong in dependencies.

},
"browserify-global-shim": {
"react": "React"
50 changes: 50 additions & 0 deletions src/usePlotly.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { useLayoutEffect, useState, useMemo } from 'react';
import { head, prop, compose, pick, objOf, mergeDeepRight } from 'ramda';
import { stream, scan } from 'flyd';

/**
* A simple debouncing function
*/
const debounce = (fn, delay) => {
let timeout;

return function (...args) {
const functionCall = () => fn.apply(this, args);

timeout && clearTimeout(timeout);
timeout = setTimeout(functionCall, delay);
};
};

const getSizeForLayout = compose(objOf('layout'), pick(['width', 'height']), prop('contentRect'), head);

export default function usePlotly() {
const updates = useMemo(stream, []);
const appendData = useMemo(stream, []);
const plotlyState = useMemo(
() => scan(mergeDeepRight, { data: [], config: {}, layout: {} }, updates),
[]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't updates be added to the dependencies list here since it's used inside the hook?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since updates is memoized with an empty array, it will only be computed once on mount, so adding it to the dependencies array or not for plotlyState will have no effect....

);

const observer = new ResizeObserver(debounce(compose(updates, getSizeForLayout), 100));
const [internalRef, setRef] = useState(null);
useLayoutEffect(() => {
if (internalRef) {
observer.observe(internalRef);
const endS = plotlyState.map(state => {
Plotly.react(internalRef, state);
});

const endAppend = appendData.map(({ data, tracePos }) => Plotly.extendTraces(internalRef, data, tracePos));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you've implemented extendTraces here, let's finish it by including maxPoints (the optional 4th argument to extendTraces)


return () => {
Plotly.purge(internalRef);
observer.unobserve(internalRef);
endAppend.end(true);
endS.end(true);
};
}
}, [internalRef, plotlyState, updates, appendData]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't observer be added as a dependency here since it's used inside the hook?


return { ref: setRef, updates, appendData };
}