Skip to content
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

Memory leak solution #5

Closed
ilansas opened this issue May 19, 2015 · 2 comments
Closed

Memory leak solution #5

ilansas opened this issue May 19, 2015 · 2 comments

Comments

@ilansas
Copy link

ilansas commented May 19, 2015

Hi,

I arrived here by looking at the issue on the react-native repo. Joshbedo gave a solution to the memory leak and I was wondering (as you didn't answered) if this was a good one and what you think about it. I just implemented it but didn't had time to run some tests.

FYI this was his answer : facebook/react-native#619 (comment)

Anyway thanks for your feedback ;)

@ilansas ilansas changed the title Memory Leak Memory leak solution May 19, 2015
@badfortrains
Copy link
Owner

Hi Ilansas,

I should probably update the readme, I wrote it when there was no cleanup code in the websocket pollyfill so closed websockets would leak memory. Since then I've fixed the issue, so I'm not currently aware of any memory leaks in the polyfill.

The facebook/react-native#619 (comment) is addressing a leak in the actual example that uses the polyfill. It looks like he is using a list view to display the messages instead of simply appending a new text view for each message, which should reduce the amount of memory needed. The point of the example is really just to show that polyfill works though, the react-native view code is naively simple and should definitely be changed in a real project.

@ilansas
Copy link
Author

ilansas commented May 28, 2015

Ok good to know. I really didn't look in deep and it's obvious that the answer here facebook/react-native#619 was for the example, haha sorry my bad.

Thanks for the feedback !

@ilansas ilansas closed this as completed May 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants