Repository navigation
v8.0.0 — zlib.DeflateRaw only extensible via class keyword #13358
Description
- Version: v8.0.0
- Platform: Darwin
- Subsystem: zlib
Extending zlib.DeflateRaw via non-class-keyword doesn't work. You can see this in action with this example:
'use strict'
const zlib = require('zlib');
const inherits = require('util').inherits;
function NotInitialized (options) {
zlib.DeflateRaw.call(this, options);
this.prop = true
}
inherits(NotInitialized, zlib.DeflateRaw);
console.log(new NotInitialized())The above prints out NotInitialized { prop: true }
What I expected it to do, was what this example does:
'use strict'
const zlib = require('zlib');
class Initialized extends zlib.DeflateRaw {
constructor (options) {
super(options)
this.prop = true
}
}
console.log(new Initialized())Not doing this initialization means that many stream operations result in crashes. (For example, pipe.)
This shows up in the real world with crc32-stream.
Activity
[-]zlib.deflateRaw not extensible via[/-][+]zlib.deflateRaw only extensible via class keyword[/+][-]zlib.deflateRaw only extensible via class keyword[/-][+]v8.0.0 — zlib.deflateRaw only extensible via class keyword[/+][-]v8.0.0 — zlib.deflateRaw only extensible via class keyword[/-][+]v8.0.0 — zlib.DeflateRaw only extensible via class keyword[/+]iarna commentedon Jun 1, 2017
This seems to be happening because
zlib.DeflateRawnow returns a new object instead of mutatingthis. While technically valid, it breaks common inheritance patterns. (As seen above.)mscdex commentedon Jun 1, 2017
/cc @jasnell
jasnell commentedon Jun 1, 2017
I'm just getting going this morning so I'll have a look a bit more in depth soon, but does this break an existing module or does it just not work as it was expected?
mcollina commentedon Jun 1, 2017
Yes, completely. I have a quick fix ready to go, a PR is coming soon.
jasnell commentedon Jun 1, 2017
Ok. Sigh, we should get those into citgm then.
mcollina commentedon Jun 1, 2017
PR with a fix in #13370.
@iarna I have not verified this with the affected module, I just checked that the example you provided passes.
iarna commentedon Jun 1, 2017
@mcollina I'll give the module I was using a go with the PR in a few hours.
jasnell commentedon Jun 1, 2017
@iarna ... I think #13374 is the direction we're most likely to go on fixing this.
zlib: revert back to Functions
zlib: revert back to Functions