Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix #952 : adding +50Mb files via createAddStream #983

Merged
merged 1 commit into from
Sep 3, 2017

Conversation

ya7ya
Copy link
Contributor

@ya7ya ya7ya commented Sep 3, 2017

Hey , this is a fix for issue #952
This uses createAddStream instead of add to add a fairly large file in the browser without crashing.

@daviddias
Copy link
Member

Although the solution should be really figuring out what is causing the mem leak, this sounds like a good interim solution.

@pgte it seems that files.add is somehow doing a memleak

Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

Thank you @ya7ya ! 🌟

@daviddias daviddias merged commit ef0ba50 into ipfs:master Sep 3, 2017
@ya7ya
Copy link
Contributor Author

ya7ya commented Sep 3, 2017

@diasdavid 👍 , Thanks. Yes this require alot more work to figure out the issue, but for use in an example it's important to keep code that doesn't crash.

I think we should keep the issue open till we nail the main cause of the mem leak.

MicrowaveDev pushed a commit to galtproject/js-ipfs that referenced this pull request May 22, 2020
Also adjust Travis CI config to perform builds and tests with node 8.

`npm install`, `npm run build`, and `npm run test` all succeeded locally with
node version `8.3.0`.

The reason for version `8.3.0` instead of `8.0.0` is that `8.3.0` introduced
support for object rest/spread, which is used by at least one of this package's
devDependencies (`go-ipfs-dep`). It may be the case that at runtime older
versions of node would work correctly with the built package, but it seems
simpler to settle on the minimum supported version being a version that can
build, test, and use the package.

Closes ipfs#983.
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.

2 participants