search

danger / danger-js

⚠️ Stop saying "you forgot to …" in code review
http://danger.systems/js/
MIT License
5.29k stars 368 forks source link

Unexpected token and octokit deprecation error #812

Open kangax opened 5 years ago

kangax commented 5 years ago

I'm running a simple example dangerfile.js straight from the docs, using the latest 7.0.2:

import { message, danger } from 'danger';
import { readFileSync } from 'fs';

const modifiedMD = danger.git.modified_files.join('- ');

message(`Changed Files in this PR: \n -  ${modifiedMD}`);

and yarn danger local throws both syntax error and deprecation notice:

> yarn danger local
yarn run v1.13.0
$ <user>/node_modules/.bin/danger local
Error: new Octokit({headers}) is deprecated. Use {userAgent, previews} instead. See https://github.com/octokit/rest.js#client-options
    at parseOptions (<user>/node_modules/@octokit/rest/lib/parse-client-options.js:43:18)
    at new Octokit (<user>/node_modules/@octokit/rest/lib/constructor.js:14:50)
    at apiForDSL (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:114:15)
    at Object.<anonymous> (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:72:23)
    at step (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:43:23)
    at Object.next (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:24:53)
    at <user>/node_modules/danger/distribution/runner/jsonToDSL.js:18:71
    at new Promise (<anonymous>)
    at __awaiter (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:14:12)
    at Object.exports.jsonToDSL (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:65:53)

Error: octokit.authenticate() is deprecated. Use "auth" constructor option instead.
    at authenticate (<user>/node_modules/@octokit/rest/plugins/authentication-deprecated/authenticate.js:4:16)
    at apiForDSL (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:116:13)
    at Object.<anonymous> (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:72:23)
    at step (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:43:23)
    at Object.next (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:24:53)
    at <user>/node_modules/danger/distribution/runner/jsonToDSL.js:18:71
    at new Promise (<anonymous>)
    at __awaiter (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:14:12)
    at Object.exports.jsonToDSL (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:65:53)
    at Object.<anonymous> (<user>/node_modules/danger/distribution/runner/jsonToContext.js:55:54)

Unable to evaluate the Dangerfile
 dangerfile.js:14
import { readFileSync } from 'fs';
       ^

SyntaxError: Unexpected token {
    at new Script (vm.js:79:7)
    at createScript (vm.js:251:10)
    at Object.runInThisContext (vm.js:303:10)
    at Module._compile (internal/modules/cjs/loader.js:657:28)
    at Object.requireFromString [as default] (<user>/node_modules/require-from-string/index.js:28:4)
    at Object.<anonymous> (<user>/node_modules/danger/distribution/runner/runners/inline.js:144:63)
    at step (<user>/node_modules/danger/distribution/runner/runners/inline.js:32:23)
    at Object.next (<user>/node_modules/danger/distribution/runner/runners/inline.js:13:53)
    at <user>/node_modules/danger/distribution/runner/runners/inline.js:7:71
    at new Promise (<anonymous>)

Error: new Octokit({headers}) is deprecated. Use {userAgent, previews} instead. See https://github.com/octokit/rest.js#client-options
    at parseOptions (<user>/node_modules/@octokit/rest/lib/parse-client-options.js:43:18)
    at new Octokit (<user>/node_modules/@octokit/rest/lib/constructor.js:14:50)
    at apiForDSL (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:114:15)
    at Object.<anonymous> (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:72:23)
    at step (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:43:23)
    at Object.next (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:24:53)
    at <user>/node_modules/danger/distribution/runner/jsonToDSL.js:18:71
    at new Promise (<anonymous>)
    at __awaiter (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:14:12)
    at Object.exports.jsonToDSL (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:65:53)
Error: octokit.authenticate() is deprecated. Use "auth" constructor option instead.
    at authenticate (<user>/node_modules/@octokit/rest/plugins/authentication-deprecated/authenticate.js:4:16)
    at apiForDSL (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:116:13)
    at Object.<anonymous> (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:72:23)
    at step (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:43:23)
    at Object.next (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:24:53)
    at <user>/node_modules/danger/distribution/runner/jsonToDSL.js:18:71
    at new Promise (<anonymous>)
    at __awaiter (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:14:12)
    at Object.exports.jsonToDSL (<user>/node_modules/danger/distribution/runner/jsonToDSL.js:65:53)
    at Object.<anonymous> (<user>/node_modules/danger/distribution/commands/utils/runDangerSubprocess.js:159:54)
Danger: ⅹ Failing the build, there is 1 fail.
## Failures
Danger failed to run `dangerfile.js`.
## Markdowns
## Error SyntaxError

Unexpected token {
dangerfile.js:14
import { readFileSync } from 'fs';
       ^

SyntaxError: Unexpected token {
    at new Script (vm.js:79:7)
    at createScript (vm.js:251:10)
    at Object.runInThisContext (vm.js:303:10)
    at Module._compile (internal/modules/cjs/loader.js:657:28)
    at Object.requireFromString [as default] (<user>/node_modules/require-from-string/index.js:28:4)
    at Object.<anonymous> (<user>/node_modules/danger/distribution/runner/runners/inline.js:144:63)
    at step (<user>/node_modules/danger/distribution/runner/runners/inline.js:32:23)
    at Object.next (<user>/node_modules/danger/distribution/runner/runners/inline.js:13:53)
    at <user>/node_modules/danger/distribution/runner/runners/inline.js:7:71
    at new Promise (<anonymous>)

### Dangerfile

---------^

Am I missing something?

orta commented 5 years ago

Interesting, do you have babel in your project? It might not be picking up the local copy and then won't be able to transpile your JS

kangax commented 5 years ago

@orta we're on babel 7.20; could this have to do with:

"resolutions": {
  "babel-core": "7.0.0-bridge.0"
},

in package.json?

jakubzitny commented 5 years ago

+1 for this. We still have old babel in our project, no resolutions.

Error: new Octokit({headers}) is deprecated. Use {userAgent, previews} instead. See https://github.com/octokit/rest.js#client-options
    at parseOptions (/app/node_modules/@octokit/rest/lib/parse-client-options.js:43:14)
    at new Octokit (/app/node_modules/@octokit/rest/lib/constructor.js:21:50)
    at apiForDSL (/app/node_modules/danger/distribution/runner/jsonToDSL.js:114:15)
    at Object.<anonymous> (/app/node_modules/danger/distribution/runner/jsonToDSL.js:72:23)
    at step (/app/node_modules/danger/distribution/runner/jsonToDSL.js:43:23)
    at Object.next (/app/node_modules/danger/distribution/runner/jsonToDSL.js:24:53)
    at /app/node_modules/danger/distribution/runner/jsonToDSL.js:18:71
    at new Promise (<anonymous>)
    at __awaiter (/app/node_modules/danger/distribution/runner/jsonToDSL.js:14:12)
    at Object.exports.jsonToDSL (/app/node_modules/danger/distribution/runner/jsonToDSL.js:65:53)

Error: octokit.authenticate() is deprecated. Use "auth" constructor option instead.
    at authenticate (/app/node_modules/@octokit/rest/plugins/authentication-deprecated/authenticate.js:4:26)
    at apiForDSL (/app/node_modules/danger/distribution/runner/jsonToDSL.js:116:13)
    at Object.<anonymous> (/app/node_modules/danger/distribution/runner/jsonToDSL.js:72:23)
    at step (/app/node_modules/danger/distribution/runner/jsonToDSL.js:43:23)
    at Object.next (/app/node_modules/danger/distribution/runner/jsonToDSL.js:24:53)
    at /app/node_modules/danger/distribution/runner/jsonToDSL.js:18:71
    at new Promise (<anonymous>)
    at __awaiter (/app/node_modules/danger/distribution/runner/jsonToDSL.js:14:12)
    at Object.exports.jsonToDSL (/app/node_modules/danger/distribution/runner/jsonToDSL.js:65:53)
    at Object.<anonymous> (/app/node_modules/danger/distribution/runner/jsonToContext.js:55:54)

Error: new Octokit({headers}) is deprecated. Use {userAgent, previews} instead. See https://github.com/octokit/rest.js#client-options
    at parseOptions (/app/node_modules/@octokit/rest/lib/parse-client-options.js:43:14)
    at new Octokit (/app/node_modules/@octokit/rest/lib/constructor.js:21:50)
    at apiForDSL (/app/node_modules/danger/distribution/runner/jsonToDSL.js:114:15)
    at Object.<anonymous> (/app/node_modules/danger/distribution/runner/jsonToDSL.js:72:23)
    at step (/app/node_modules/danger/distribution/runner/jsonToDSL.js:43:23)
    at Object.next (/app/node_modules/danger/distribution/runner/jsonToDSL.js:24:53)
    at /app/node_modules/danger/distribution/runner/jsonToDSL.js:18:71
    at new Promise (<anonymous>)
    at __awaiter (/app/node_modules/danger/distribution/runner/jsonToDSL.js:14:12)
    at Object.exports.jsonToDSL (/app/node_modules/danger/distribution/runner/jsonToDSL.js:65:53)
Error: octokit.authenticate() is deprecated. Use "auth" constructor option instead.
    at authenticate (/app/node_modules/@octokit/rest/plugins/authentication-deprecated/authenticate.js:4:26)
    at apiForDSL (/app/node_modules/danger/distribution/runner/jsonToDSL.js:116:13)
    at Object.<anonymous> (/app/node_modules/danger/distribution/runner/jsonToDSL.js:72:23)
    at step (/app/node_modules/danger/distribution/runner/jsonToDSL.js:43:23)
    at Object.next (/app/node_modules/danger/distribution/runner/jsonToDSL.js:24:53)
    at /app/node_modules/danger/distribution/runner/jsonToDSL.js:18:71
    at new Promise (<anonymous>)
    at __awaiter (/app/node_modules/danger/distribution/runner/jsonToDSL.js:14:12)
    at Object.exports.jsonToDSL (/app/node_modules/danger/distribution/runner/jsonToDSL.js:65:53)
    at Object.<anonymous> (/app/node_modules/danger/distribution/commands/utils/runDangerSubprocess.js:159:54)
Danger: ✓ passed review, received no feedback.
f-meloni commented 5 years ago

The deprecation message is visible also on danger-swift's CI

https://travis-ci.org/danger/swift/jobs/487507560#L414

kangax commented 5 years ago

Just tried this with 7.0.9 and there're no more deprecation warnings but the syntax error is still there:

Unexpected token {
dangerfile.js:14
import { readFileSync } from 'fs';

Any ideas on how to fix this?

orta commented 5 years ago

I think this is something to do with how the automatic babel setup is working on your project, try run DEBUG="*" danger pr [a pre url]and see what it says about your babel stack

kangax commented 5 years ago

@orta Thanks! The relevant output seems to be:

2019-02-12T18:45:39.679Z danger:inline_runner Handling custom module:  /Users/jzaytsev/dev/spacestation-v2/babel.config.js

2019-02-12T18:45:39.680Z babel:config:loading:files:configuration Auto-ignoring usage of config '/Users/jzaytsev/dev/spacestation-v2/babel.config.js'.

2019-02-12T18:45:39.680Z babel:config:loading:files:configuration Found root config 'babel.config.js' in $o. /Users/jzaytsev/dev/spacestation-v2

2019-02-12T18:45:39.692Z babel:config:loading:files:plugins Loaded plugin '@babel/plugin-transform-flow-strip-types' from '/Users/jzaytsev/dev/spacestation-v2'.

2019-02-12T18:45:39.692Z babel:config:loading:files:configuration Auto-ignoring usage of config '/Users/jzaytsev/dev/spacestation-v2/babel.config.js'.

2019-02-12T18:45:39.692Z babel:config:loading:files:configuration Found root config 'babel.config.js' in $o. /Users/jzaytsev/dev/spacestation-v2

so it finds config but then ignores it?

Full output

maniator commented 5 years ago

It seems after working with @kangax on this that regular node module imports work fine (like 'lodash', and 'danger'), but native imports like fs and path, it seems as though you have to use require('module') and not use import module from 'module' as then you hot the error that @kangax had.

I will try to look into the danger code to see if there is anything I can do for a PR to fix this issue