ActiveCampaign / activecampaign-api-nodejs

Node.js wrapper for the ActiveCampaign API
MIT License
39 stars 36 forks source link

Invalid instance in docs #43

Closed leebenson closed 6 years ago

leebenson commented 7 years ago

In the readme, you have:

new ActiveCampaign("https://ACCOUNT.api-us1.com", {{KEY}});

In ES6, {KEY} is equivalent to { KEY: KEY }, which would lead me to believe that the right way to create a new instance would be something like:

const ac = new ActiveCampaign('https://account.api-us1.com', {
    KEY: 'some key',
});

When instead, it's actually:


const ac = new ActiveCampaign('https://account.api-us1.com', 'some key');
bartboy011 commented 6 years ago

Hey @leebenson, the example provided is Handlebar syntax, ie, there are two brackets present, not one. This was to suggest that this was a value that needed to be replaced in your implementation.

leebenson commented 6 years ago

IMO that's ambiguous/unclear, since {{KEY}} is valid JS if KEY is a defined value.

Would be better to surround KEY in quotes, to represent the string the constructor is expecting.

bartboy011 commented 6 years ago

@leebenson We'll be rewriting the README for the next major release and will adjust this so that it's more clear. However, double brackets {{}} are not valid JS syntax. You're right that that would be valid syntax if it were {KEY} and that can lead to confusion so we'll make that change for sure.

leebenson commented 6 years ago

it's valid in this context:

someFunction = KEY => {{KEY}}

The first curly brace denotes a block; the second is an object literal. Nothing is returned in this example because the object is used as a statement, but it still runs.

My point is: It's ambiguous to see it in a readme.