Skip to content
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

(#3877) - refactor binary #3877

Closed
wants to merge 1 commit into from
Closed

(#3877) - refactor binary #3877

wants to merge 1 commit into from

Conversation

nolanlawson
Copy link
Member

Move them out of utils.js and into their own separate files.
None of this is a functional change, except for a small optimization
I noticed in request-browser.js, where we can directly use
readAsArrayBuffer instead of first converting to a binary string.

@@ -52,8 +53,8 @@ function fetchRequest(options, callback) {
}

if (options.body && (options.body instanceof Blob)) {
utils.readAsBinaryString(options.body, function(binary) {
fetchOptions.body = utils.fixBinary(binary);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's where we can simplify by just directly using readAsArrayBuffer.

@nolanlawson
Copy link
Member Author

As for file naming conventions, I think we should start moving away from hyphenated-names-like-this.js, at least internally. It looks weird when you're requiring a function, e.g. var readAsArrayBuffer = require('read-as-array-buffer'). I also noticed that libraries like lodash use camel case for inner modules, which is why you can do nice things like require('lodash/array/zipObject').

@nolanlawson
Copy link
Member Author

Another note: this does increase the min+gz size from 47655 to 47725, probably due to the extra browserify boilerplate around each little function.

testUtils.atob = function (arg) {
return PouchDB.utils.atob(arg);
};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to reduce our dependence on PouchDB.utils in the tests.

@nolanlawson
Copy link
Member Author

forgot the commit message format. we can fix on the way in

Move them out of `utils.js` and into their own separate files.
None of this is a functional change, except for a small optimization
I noticed in `request-browser.js`, where we can directly use
`readAsArrayBuffer` instead of first converting to a binary string.
@nolanlawson nolanlawson changed the title refactor binary (#3877) - refactor binary May 27, 2015
nolanlawson added a commit that referenced this pull request May 27, 2015
Move them out of `utils.js` and into their own separate files.
None of this is a functional change, except for a small optimization
I noticed in `request-browser.js`, where we can directly use
`readAsArrayBuffer` instead of first converting to a binary string.
nolanlawson added a commit that referenced this pull request May 27, 2015
Move them out of `utils.js` and into their own separate files.
None of this is a functional change, except for a small optimization
I noticed in `request-browser.js`, where we can directly use
`readAsArrayBuffer` instead of first converting to a binary string.
@daleharvey
Copy link
Member

all makes sense, cheers - 2376abe

@daleharvey daleharvey closed this May 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants