Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

fix: refresh cache memory leak example #784

Merged
merged 2 commits into from
Jan 2, 2018

Conversation

samouss
Copy link
Collaborator

@samouss samouss commented Jan 2, 2018

Summary

Clear the interval on component willUnmount.

@samouss samouss requested review from Haroenv and marielaures January 2, 2018 08:04
@samouss samouss force-pushed the fix/refresh-cache-memory-leak-example branch from d316bf7 to 6202bc6 Compare January 2, 2018 08:07
@algobot
Copy link
Contributor

algobot commented Jan 2, 2018

Deploy preview ready!

Built with commit 9f89076

https://deploy-preview-784--react-instantsearch.netlify.com

@algolia algolia deleted a comment from algobot Jan 2, 2018
@samouss samouss force-pushed the fix/refresh-cache-memory-leak-example branch from 6202bc6 to 9f89076 Compare January 2, 2018 08:23
@@ -82,7 +82,11 @@ class App extends Component {
}

componentDidMount() {
setInterval(() => this.setState({ refresh: true }), 5000);
this.interval = setInterval(() => this.setState({ refresh: true }), 5000);
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT of setting line length in markdown files to something like 60? I prefer having shorter lines when reading something in a post, rather than while writing code

Copy link
Collaborator Author

@samouss samouss Jan 2, 2018

Choose a reason for hiding this comment

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

Agree, I'm looking for something like that yesterday 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll need two prettier configs for that, not sure if it's possible already 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we can, printWidth also handle markdown by default.

https://prettier.io/docs/en/options.html#print-width

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant that we can't have js: { printWidth: 80 }, md: { printWidth: 55 } IIRC, but would be good to add

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah ok. Yep would be great to support this kind of configuration!

@samouss samouss merged commit cf228ac into master Jan 2, 2018
@samouss samouss deleted the fix/refresh-cache-memory-leak-example branch January 2, 2018 09:26
samouss added a commit that referenced this pull request Jan 3, 2018
<a name="4.4.0"></a>

* **createInstantSearch:** remove the client from the Snapshot ([#749](#749)) ([700d8f4](700d8f4))
* refresh cache memory leak example ([#784](#784)) ([cf228ac](cf228ac))
* **stories:** rename InstantSearch to <InstantSearch> ([#789](#789)) ([05efda5](05efda5))

* InstantSearch root props ([#770](#770)) ([2d458f8](2d458f8))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants