Skip to content

Throw warning for unsupported runtimes, e.g. Node < 6 #4839

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

Merged

Conversation

GeoffreyBooth
Copy link
Collaborator

Running coffee or cake in Node 4 or below results in an unhelpful stack trace (per here):

user@computer:~/some/path $ coffee -c code.coffee 
/usr/local/lib/node_modules/coffeescript/lib/coffeescript/command.js:23
  ({spawn, exec} = require('child_process'));
   ^

ReferenceError: Invalid left-hand side in assignment
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:374:25)
    at Object.Module._extensions..js (module.js:417:10)
    at Module.load (module.js:344:32)
    at Function.Module._load (module.js:301:12)
    at Module.require (module.js:354:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/usr/local/lib/node_modules/coffeescript/bin/coffee:15:5)
    at Module._compile (module.js:410:26)
    at Object.Module._extensions..js (module.js:417:10)

We can do better than this. This minor addition first checks that the runtime supports destructuring, and throws an informative error if your runtime doesn’t:

user@computer:~/some/path $ coffee -c code.coffee
Your JavaScript runtime does not support some features used by the coffee command.
Please use Node 6 or later.

Destructuring is really just a proxy for all the advanced ES2015 features that the CoffeeScript module uses, but it should be enough. We already use CI to test that CoffeeScript works in Node 6, and destructuring isn’t supported in Node 4 and below, so this should catch all the unsupported runtimes.

@GeoffreyBooth GeoffreyBooth force-pushed the warn-unsupported-runtime branch 2 times, most recently from 757d99f to 9e79d31 Compare December 31, 2017 23:38
@GeoffreyBooth GeoffreyBooth force-pushed the warn-unsupported-runtime branch from 9e79d31 to d3a74b5 Compare January 12, 2018 18:49
@GeoffreyBooth
Copy link
Collaborator Author

Can someone who’s more an expert in the coffee command and launch stuff please review this? It’s very small. @lydell?

@lydell
Copy link
Collaborator

lydell commented Jan 12, 2018

Looks like a working solution to me. But I can't really know how safe it is.

Any particular reason you didn't use eval directly?

@GeoffreyBooth
Copy link
Collaborator Author

Any particular reason you didn’t use eval directly?

Well, mainly because it’s been drilled into me that eval is evil, so therefore anything else must be better. More practically, new Function creates its own scope whereas eval uses the scope it’s in, so this PR doesn’t end up creating a new a variable. I guess that’s as good a reason as any to prefer new Function, unless there’s some reason you think I should use eval.

@lydell
Copy link
Collaborator

lydell commented Jan 15, 2018

Thanks for the explanation. Sounds good.

@GeoffreyBooth GeoffreyBooth merged commit a70f1a0 into jashkenas:master Jan 15, 2018
@GeoffreyBooth GeoffreyBooth deleted the warn-unsupported-runtime branch January 15, 2018 07:40
zdenko pushed a commit to zdenko/coffeescript that referenced this pull request Jan 17, 2018
GeoffreyBooth pushed a commit that referenced this pull request Feb 1, 2018
…ariables (#4853)

* fix #2047

* Additional check for 'step'; tests

* Fix #4105 (#4855)

* Update output

* Throw warning for unsupported runtimes, e.g. Node < 6 (#4839)

* fix #1403 (#4854)

* Update output

* [Change]: Destructuring with non-final spread should still use rest syntax (#4517) (#4825)

* destructuring optimization

* refactor

* minor improvement, fix errors

* minor refactoring

* improvements

* Update output

* Update output

* Fix #4843: bad output when assigning to @prop in destructuring assignment with defaults (#4848)

* fix #4843

* improvements

* typo

* small fix

* Fix #3441: parentheses wrapping expression throw invalid error  (#4849)

* fix #3441

* improvements

* refactor

* Fix #1726: expression in property access causes unexpected results (#4851)

* fix #1726

* Explain what's happening, rather than just linking to an issue

* Updated output

* Optimization

* Update output

* remove casting to number

* cleanup tests
@GeoffreyBooth GeoffreyBooth mentioned this pull request Feb 1, 2018
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