-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
Probably should init the ipfs repo by itself. |
@victorbjelkholm great catch! completely escaped my eye |
#723 (comment) |
On the With alias of zlib-browserify-zlib-next Uncaught TypeError: Cannot read property 'toString' of undefined
at Object.eval (webpack:////Users/koruza/code/js-ipfs/~/browserify-zlib-next/lib/index.js?:9 Without alias of zlib-browserify-zlib-next Uncaught TypeError: function Buffer(arg) {
if (!(this instanceof Buffer)) {
// Avoid going through an ArgumentsAdaptorTrampol...<omitted>...
} is not a function
at Function.from (native)
at toBn (webpack:////Users/koruza/code/js-ipfs/~/libp2p-crypto/src/crypto/util.js?:19:24) |
@diasdavid just pushed a fix for this, to this branch. We should probably put a note somewhere that we require ^webpack@2 |
f5e24fc
to
b7ce4a7
Compare
Thank you! Added a note of that to the Example and to the README. |
const fs = require('fs') | ||
const os = require('os') | ||
const series = require('async/series') | ||
const IPFS = require('../../src/core') // replace this by line below |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"replace this by line below" should probably be expanded to "replace this by line below if you are running this example from outside the js-ipfs repository" just to be explicit about why you would replace it.
/* | ||
* Create a new IPFS instance, using default repo (fs) on default path (~/.ipfs) | ||
*/ | ||
const node = new IPFS(os.tmpDir() + '/' + new Date().toString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use path.join
instead of concatenating strings
* https://github.com/ipfs/interface-ipfs-core/tree/master/API/files | ||
*/ | ||
(cb) => { | ||
if (node.isOnline()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the node is not online after doing goOnline
, there must have been an error somewhere, no? We should either not need to have this here, or if we do, not add the file if we're not online.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not mandatory to be online to add the files. But get what you say, I basically preserved this operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sure, but in the previous step, we do go online, so either we're online here, or something bad happened.
|
||
console.log('\nAdded file:') | ||
console.log(result[0]) | ||
fileMultihash = result[0].hash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of doing mutation on the fileMultihash variable, you should be able to pass the value as a second parameter to the callback and then retrieve it in the next step, if I remember correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, I'm not a big fan of that, because then the tasks can't be easily swapped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, all right, makes sense 👍
## Special note | ||
|
||
In order to use js-ipfs in the browser, you need to replace the default `zlib` library by `browserify-zlib-next`, a full implementation of the native `zlib` package, full in Node.js. | ||
See the package.json to learn how to do this and avoid this pitfall. More context on: https://github.com/ipfs/js-ipfs#use-in-the-browser-with-browserify-webpack-or-any-bundler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"see the package.json" for what? I'm not sure but probably you mean "see the webpack.config.js" for aliasing zlib to browserify-zlib-next no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
absolutely! 👍
@@ -0,0 +1,7 @@ | |||
'use strict' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're bundling this with webpack and stage-0 + react, files are already always running in strict mode.
No description provided.