Skip to content
This repository has been archived by the owner on Jan 12, 2021. It is now read-only.

Better handling of boolean arguments #24

Merged
merged 2 commits into from
Aug 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ const cli = meow(`
string: [
'app-version',
],
boolean: [
'overwrite',
'upload-node-modules',
'upload-sources',
'add-wildcard-prefix',
],
});

const conf = {
Expand Down
8 changes: 8 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,14 @@ function prepareRequest(options) {
case 'tempDir': {
break;
}
case 'overwrite': {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this only apply to overwrite, and not any of the other boolean flags?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only overwrite as that's the only one that gets sent to the API. All of the other boolean args configure the behaviour of this module but don't get put in the payload.

// the presence of any value for this flag causes the API to interpret it as
// true, so only add it to the payload if it is truthy
if (options.overwrite) {
formData[name] = String(value);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If I make a curl call with overwrite=false, would the API interpret that value as truthy? If so, we should update our docs to make that clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

break;
}
// Basic fields (strings/booleans) & future fields
default: {
formData[name] = String(value);
Expand Down
10 changes: 10 additions & 0 deletions index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const stripProjectRoot = require('./index').stripProjectRoot
const upload = require('./index').upload
const validateOptions = require('./index').validateOptions
const prepareRequest = require('./index').prepareRequest

test('upload function exists', () => {
expect(typeof upload).toBe('function');
Expand Down Expand Up @@ -71,3 +72,12 @@ describe('validateOptions', () => {
}).toThrow('You must provide a path to the source map you want to upload.');
});
});

describe('prepareRequest', () => {
test('removes options.overwrite when false', () => {
expect(prepareRequest({ overwrite: false }).formData).toEqual({});
});
test('does not remove options.overwrite when true', () => {
expect(prepareRequest({ overwrite: true }).formData).toEqual({ overwrite: 'true' });
});
});