| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
sigh, this spurs a linter error: lib/util.js
665:24 error Use const Buffer = require('buffer').Buffer; at the beginning of this file require-buffer
✖ 1 problem (1 error, 0 warnings)
cc @silverwind |
Sorry, something went wrong.
|
You can also put this line before the requires |
Sorry, something went wrong.
|
Still hacky, but better! Thanks @targos, I switched the order of exports. I tried to remove the isBuffer call altogether (keep it DRY with buffer.isBuffer), but that causes another circular dependency. I think the best way forward is to move util.deprecate to an internal module. |
Sorry, something went wrong.
|
Alright, force pushed a fix that moves deprecate() to an internal module, allowing us to circumvent the whole thing. Right now, the internal module .deprecate is still exposed through util.deprecate. |
Sorry, something went wrong.
|
@brendanashworth #1892 should fix that eventually. As for the linter error, you could just put /* eslint-disable require-buffer */ in worst case. |
Sorry, something went wrong.
|
Also I wonder why no test has caught this circular dependency. util.isBuffer seems completely untested, maybe add a small test? |
Sorry, something went wrong.
|
@silverwind hmm, looking at #1892, it would reintroduce the dep issue because the internal module references back to the util module. Do you think I should just add the linter error hack for now, have #1892 fix the cyclic issue, and call it a day? And I added a small true / false test, I too was surprised it wasn't caught by the test suite. Added a small commit, summary: > util.isBuffer === Buffer.isBuffer
true |
Sorry, something went wrong.
|
I'd be fine with a hack if you denote it as such (// HACK:). #1892 still needs a bit of work. |
Sorry, something went wrong.
|
I'd prefer to go through with these commits, without introducing a hack / linter comment, to fix the bug. #1892 shouldn't be hard to port the changes through, as the location of the code is all that changes. |
Sorry, something went wrong.
|
True, shouldn't be too hard to have separate methods later. LGTM, started off a CI just to be sure: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/17/ |
Sorry, something went wrong.
|
CI failures are unrelated. @brendanashworth can you land it? This and 671e64a are pretty important fixes and we should probably do a quick patch release for them. |
Sorry, something went wrong.
|
@silverwind yes, landing now - 3806d87 seems to be important too, they were talking about a CVE. |
Sorry, something went wrong.
PR-URL: #1988 Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: #1988 Reviewed-By: Roman Reiss <me@silverwind.io>
|
Thank you, I've merged these commits in 671e64a...626432d, in time for #1996. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Importing Buffer caused a circular dependency issue. We need to use the
globally exported variable to get around this.
Fixes: #1987
This is just quick patch to fix the bug, I hope we can later find a better solution.