CondeNast / perf-timeline-cli

Generate Chrome Performance Timelines via a command line interface
Apache License 2.0
103 stars 4 forks source link

Generate command crashes on node 8.1.2 #10

Closed alexnm closed 6 years ago

alexnm commented 6 years ago

Current Behavior

I was looking forward to testing this tool, but unfortunately it crashes when I run the example generate command

I get the following error

/Users/alex/.nvm/versions/node/v8.1.2/lib/node_modules/@condenast/perf-timeline-cli/src/commands/generate/handler.js:71
  const { url, ...options } = argv;
               ^^^

SyntaxError: Unexpected token ...
    at createScript (vm.js:74:10)
    at Object.runInThisContext (vm.js:116:10)
    at Module._compile (module.js:533:28)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/Users/alex/.nvm/versions/node/v8.1.2/lib/node_modules/@condenast/perf-timeline-cli/src/commands/generate.js:2:21)

I was thinking it's a node problem, but the rest operator should be supported. Am I doing something wrong?

Your Environment

I have a Mac OS-X with nvm installed and set to node 8.1.2.

Thanks

tollmanz commented 6 years ago

According to a Stack Overflow comment, spread operators are supported from Node 8.3. I cannot back this claim up with support charts, but in testing, I find that 8.0.x, 8.1.x, and 8.2.x throw this error. 8.3.0 does not.

If you want to test this tool right now, you can install 8.3+. I have confirmed this as a bug and will work on a fix for < 8.3.

Thanks so much for taking the time to report the error!

tollmanz commented 6 years ago

Ah...Node 8.3 upgraded V8 to version 6. V8 version 6 included:

This release introduces rest properties for object destructuring assignment and spread properties for object literals.

This accounts for the error that you were seeing.

tollmanz commented 6 years ago

@alexnm Thanks again for reporting this! The bug is addressed and I've added testing environments for lesser versions of Node 8 to hopefully catch these issues earlier in the future. You can update to the latest with npm i -g @condenast/perf-timeline-cli. Again, I really appreciate you trying the tool and reporting your experiences.

alexnm commented 6 years ago

@tollmanz sure, thanks for the super quick fix, working as expected now! I'm more used to FE work and I'm not sure how the different node versions support the native features from ES20XX. Looking forward to test the tool soon