search

MadKudu / node-hubspot

Node wrapper for the HubSpot API
MIT License
192 stars 157 forks source link

Broadcast.js throws syntax error in 1.3.4 due to comma after cb #151

Closed hughstephens closed 5 years ago

hughstephens commented 5 years ago

Hi team

Just a minor suggestion - v1.3.4 updated automatically for us from NPM, and then we got a series of errors (copy below). I think it's the trailing comma @ https://github.com/MadKudu/node-hubspot/blob/1.3.4/lib/broadcast.js#L18

Suggest maybe unpublishing v1.3.4 rather than keep it live? Just to avoid others having same problem.

/var/app/current/node_modules/hubspot/lib/broadcast.js:19 ) ^ SyntaxError: Unexpected token ) at Object.exports.runInThisContext (vm.js:73:16) at Module._compile (module.js:543:28) at Object.Module._extensions..js (module.js:580:10) at Module.load (module.js:488:32) at tryModuleLoad (module.js:447:12) at Function.Module._load (module.js:439:3) at Module.require (module.js:498:17) at Module.require (/var/app/current/node_modules/require-in-the-middle/index.js:43:24) at require (internal/module.js:20:19) at Object. (/var/app/current/node_modules/hubspot/lib/client.js:1:81)

hughstephens commented 5 years ago

(Suggesting just unpublishing that version given you're at 2.0.0 now, but this way others don't have to spend the time debugging working out what's going on like I just did…)

MattMSumner commented 5 years ago

Given we have tests for broadcast, I'm surprised this wasn't caught. Which version of node are you running?

MattMSumner commented 5 years ago

@hughstephens looks like trailing commas in functions calls is supported in node 8.11.1 per this issue: https://github.com/nodejs/node/issues/11258.

I agree that pulling the version is an option. If I get some time I could open a PR changing the prettier setting to not add trailing commas to functions calls.

@pcothenet do you have any thoughts?

hughstephens commented 5 years ago

I think that env is still on 7.6.0, which is probably the cause. Our main env is 8.11.3 but haven't tested in there (we just locked to v1.3.3 in package.json).

Not a major issue etc, but just always fun when npm helpfully* upgrades packages and everything breaks...

BrownieBits commented 5 years ago

Can I remove the commas locally and deploy or does the deployed content git the published version from npm?