-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
add support for node 4.2.0 to 4.4.7 with specific buffer polyfill #665
add support for node 4.2.0 to 4.4.7 with specific buffer polyfill #665
Conversation
Hmm, what if we'd just change the polyfill to account for this: if (!Buffer.from || Buffer.from === Uint8Array.from)
Buffer.from = function from(value, encoding) { return new Buffer(value, encoding); }; This of course assumes that nobody is intentionally using broken |
@dcodeIO Hmm, yeah I like that method better, but I got
when I tried to replace Buffer.from. Seems strict mode prevents us from doing so? |
Well, I assume your initial approach is the right one then. Going to get a node version for testing. |
…ther: Added fetch test cases + some test cleanup
This commit changes how Buffer.from / Buffer.allocUnsafe are accessed. These are now private properties on Also added node 4.3.2 to Travis as you suggested. |
Nice refactor 👍 Closing this PR since master works for me now locally and on Lambda. I also ran the test suite on node 4.0.0 and 4.1.2, and all tests pass -- so I believe you can change your comment to support all 4.x versions since you've refactored around the Buffer inheritance bug in the earlier versions that I wasn't able to solve. |
This PR enables support for Node 4.3.2 (which I'm stuck with on AWS Lambda).
The
Buffer
object in Node 4.0, 4.1, 4.1, and 4.4 have aBuffer.from
method defined which is inherited fromUint8Array
. The current polyfill only checks iffrom
is present and thefrom
method in these versions accept a typed array as an argument, which doesn't behave nicely.This PR replaces
util.Buffer
with a new object on those node versions and brings compatibility to Node 4.2.0 to 4.4.7. Unfortunately,Buffer
can't be inherited from on 4.0 and 4.1, so this polyfill doesn't work there.Ran tests locally with node 4.3.2 and they all passed and I'm also using this PR to decode protobuf messages in lambda through the static code generation.
Maybe node 4.3.2 should be added to travis to prevent regression?