search

HaxeFoundation / hxnodejs

Haxe externs for working with node.js
MIT License
171 stars 63 forks source link

add missing methods which are causing issues in haxe.zip.Reader #160

Closed peteshand closed 4 years ago

Gama11 commented 4 years ago

Does this really help with anything? Looks like it will just lead to a runtime error as soon as you call the constructor.

peteshand commented 4 years ago

Yeah it's stops a compile error.

In the std lib, haxe.zip.Reader calls new Uncompress, which throws an error. This bug was reported quite a while ago, so would be good to resolve it

Gama11 commented 4 years ago

But it doesn't look like this actually solves anything, it just moves the error from compile time to runtime?

peteshand commented 4 years ago

Hmm that's not entirely true, It moves the error from always happening at compile time, to only happening when you call a particular method on Reader. For example I'm not using Reader, but because of this in compatibility my whole app won't compile. And if you are using Reader then your no worse off, as your app already won't be compiling.

Gama11 commented 4 years ago

Hm, I see... Do you have a code example?

peteshand commented 4 years ago

It's GMT+10 so not at my computer. But basically hxnodejs overrides a std class and removes a bunch of methods, this is going to cause issues, shouldn't need a code example to see this. Could you just merge please

Gama11 commented 4 years ago

Calm down. :) It's not about understanding, I was thinking it would make for a good test case for unit tests / CI. But it looks like I misremembered and we don't actually have unit tests for hxnodejs, just these examples.

nadako commented 4 years ago

It's probably fine to merge this, tho for Uncompress you can probably use this code.

peteshand commented 4 years ago

Thanks