-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Uncaught Error: Can't set property: 'links' is immutable #1131
Uncaught Error: Can't set property: 'links' is immutable #1131
Comments
@haadcode that property has been immutable for a while https://github.com/ipld/js-ipld-dag-pb/blob/master/src/dag-node/index.js#L51-L53 wonder if you changed something in your codebase that tries to change the links property. You can still use the class functions as you could before, such as |
I’ve caught this one too, and it happens if I have no Swarm addresses set.
…On 7 Dec 2017, 18:32 +0000, David Dias ***@***.***>, wrote:
@haadcode that property has been immutable for a while https://github.com/ipld/js-ipld-dag-pb/blob/master/src/dag-node/index.js#L51-L53 wonder if you changed something in your codebase that tries to change the links property.
You can still use the class functions as you could before, such as .addLink, see docs at https://github.com/ipld/js-ipld-dag-pb#table-of-contents
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
This only happens with a minified build of ipfs ( |
If I pull js-ipfs from master, build it and
Produces
And using @diasdavid perhaps something went funky in the release and the up-to-date dist file was not included? |
Seems that #1136 also had issues. Let me try to publish again. |
@haadcode could you try with latest? |
@diasdavid I can confirm that it still happens with 0.27.3 :/ Using However, I have good news, I think :) I was able to get everything working correctly with webpack, compiling and uglifying orbitdb and ipfs together, by setting the const uglifyOptions = {
uglifyOptions: {
mangle: false,
compress: false,
},
} When uglified non-compressed, it seems to work as expected while still minifying the output bundle. So what I take from this is that in order to fix |
I'm using 0.27.4 from jsdelivr cdn and this error was thrown. Sample code:
|
hey, this was fixed in a quick way , which is not to |
This is part of ongoing effort to properly fix ipfs/js-ipfs#1131.
Replace constructor.name with instanceof. This is part of ongoing effort to properly fix ipfs/js-ipfs#1131.
Thanks @ya7ya for your pull requests. The problem is that using Do I understand correctly that this issue happens when you bundle js-ipfs with your own project an minify it? So it doesn't happen if you use the standalone minified version? If that's the case then we should document in the README which options you need to set on uglifyjs if you want to minify it together with your project. Credit where credit is due: Thanks @dryajov and @dignifiedquire for looking into this. |
@vmx note the bad merge here multiformats/js-multiaddr#55 |
A workaround that would allow for minifying while not breaking anything in the non-minified versions would be two do both checks, and replace the pattern
with
It is not very pretty, and still would mean that there would remain to be a difference in behavior after minitifying (but less than before), but for us, but at least it would allow for minifying, and it would not break anything existing afaics. |
Wouldn't the safest workaround be to gather a list of classes that do such a check and excluding those from the minifaction via an UglifyJS setting? |
@vmx - wont you run into other problems with constructor.name, i.e. that anyone subclassing is going to fail your test? @jellegerbrandy's solution looks like a good one, partly since someone including js-ipfs into their project might not know much about the IPFS constraint on not minifying at all, especially if they include something that includes IPFS. (For example our archive.org demo page requires our Dweb library which requires IPFS, currently I'm maintaining all three, but that's unlikely to be the case once the project scales), and coincidentally it was starting to concern me that things that work in my development version (not minified) failed in IPFS in the 'production' (i.e. minified) version. |
@vmx @mitra42 @jellegerbrandy What about creating an Something like this: const typeSymbol = Symbol.for('whatever-type-symbol');
class Whatever {
static isWhatever(obj) {
return obj && Boolean(obj[typeSymbol]);
}
constructor() {
this[typeSymbol] = true;
}
}
const whatever = new Whatever();
Whatever.isWhatever(whatever); // true |
Check #1321 for more info. |
Is uglify working in the current (0.29.3) js-ipfs yet. Its hard to tell because the breakages are subtle. |
It is @mitra42. But if you have a custom webpack config, you have to pass this option to UglifyJS: compress: {
unused: false
} |
@fsdiogo - sorry, but but unclear on this, what do you mean a "custom webpack config", I didn't think webpack worked without a config. Am I assuming i can place this at the top level of the webpack config, or does it somehow need passing to UglifyJS. |
Ignore the custom part, I was referring to your webpack config. You have to pass the Check this comment for reference, but if you're using compress: {
unused: false
} |
Thanks - for the pointer to that comment, so its still unclear whether if you do not explicitly use Uglify (i.e. just use "webpack --mode production") you still need to add.
|
Without that code (I don't actually use uglify) then I see a lot of Its possible of course that I'm doing something wrong, but it would be interesting if anyone else is seeing these on the minified IPFS. |
If you use That's why you are seeing the |
Replace constructor.name with instanceof. This is part of ongoing effort to properly fix ipfs/js-ipfs#1131.
Has anyone else been able to follow the notes here and get Uglify to work ?
to webpack.config results in
|
Try: const UglifyJsPlugin = require('uglifyjs-webpack-plugin');
...
plugins: [
new UglifyJsPlugin({
uglifyOptions: {
compress: {
unused: false
}
}
})
] |
Thanks @alanshaw that worked, It would probably a good idea to add that configuration to https://github.com/ipfs/js-ipfs/blob/master/examples/browser-webpack/webpack.config.js . |
@mitra42 what about submitting that as a PR? ;) |
Sure ... about half way through doing this, I realized that the browser-webpack example is actually a browser-webpack-react example, and while it works with the builtin webpack server, it wont actually compile with the usual ways of building a bundle that can be included in a project, Does this need splitting into two examples, with one just showing how people can include IPFS in their (compressed webpack) projects without hitting this bug ? |
Yes, a webpack example without framework will allow people to use it as a base and add their own framework. Many people will be using webpack+react, but not eveyone! |
After updating to 0.27.0 and running in the browser, there's an error that didn't happen before:
Version: 0.27.0
Platform: Browser
This happens upon starting IPFS, but it never starts as the error is thrown. Doesn't happen with 0.26.0.
Full stack trace for what it's worth (index.min.js is
ipfs/dist/index.min.js
):The text was updated successfully, but these errors were encountered: