-
Notifications
You must be signed in to change notification settings - Fork 44
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
Use dignified.js #12
Use dignified.js #12
Conversation
"webpack": { | ||
"resolve": { | ||
"alias": { | ||
"node-forge": "../vendor/forge.bundle.js" |
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.
nice! Was looking for a note on how to add specifics like this :)
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.
This is currently only possible for webpack
. When the need arises we can add it for others as well
|
// protobuf read from file | ||
const messages = isNode ? protobuf(fs.readFileSync(path.resolve(__dirname, 'pb/crypto.proto'))) : protobuf(require('buffer!./pb/crypto.proto')) | ||
const messages = protobuf(fs.readFileSync(path.resolve(__dirname, '../protos/crypto.proto'))) |
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.
This seems to be braking the browser tests on js-ipfs
. Why did you remove the isNode
check?
cc @dignifiedquire @diasdavid
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.
Because this way we can use brfs
to always load the file. As soon as js-ipfs
uses dignified.js it should work again, or you can manually add this config: https://github.com/dignifiedquire/dignified.js/blob/master/config/webpack.js#L89-L92
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.
Got it, thanks @dignifiedquire!
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.
Did the tests not run for you @xicombd ?
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.
No, js-ipfs
's browser tests before ipfs/js-ipfs#132 were not passing.
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.
got it :)
Ref ipfs/aegir#1