decaffeinate / bulk-decaffeinate

Run decaffeinate and related operations on a whole codebase, or just part of one.
MIT License
74 stars 13 forks source link

Cannot read property 'messages' of undefined #128

Open hefox opened 7 years ago

hefox commented 7 years ago

When I try to bulk-decaffeinate some files, I get this error:

Running eslint --fix on all files.. TypeError: Cannot read property 'messages' of undefined at runEslint$ (/usr/local/lib/node_modules/bulk-decaffeinate/dist/bulk-decaffeinate.js:2590:38) at tryCatch (/usr/local/lib/node_modules/bulk-decaffeinate/node_modules/regenerator-runtime/runtime.js:65:40) at GeneratorFunctionPrototype.invoke [as _invoke] (/usr/local/lib/node_modules/bulk-decaffeinate/node_modules/regenerator-runtime/runtime.js:303:22) at GeneratorFunctionPrototype.prototype.(anonymous function) [as next] (/usr/local/lib/node_modules/bulk-decaffeinate/node_modules/regenerator-runtime/runtime.js:117:21) at tryCatch (/usr/local/lib/node_modules/bulk-decaffeinate/node_modules/regenerator-runtime/runtime.js:65:40) at invoke (/usr/local/lib/node_modules/bulk-decaffeinate/node_modules/regenerator-runtime/runtime.js:155:20) at /usr/local/lib/node_modules/bulk-decaffeinate/node_modules/regenerator-runtime/runtime.js:165:13

Tried with both latest version of eslint and 4.0 rc, no happens both times. Will update if I get any more info on it

alangpierce commented 7 years ago

Thanks for reporting! Looks like eslint is producing some output that bulk-decaffeinate can't parse. bulk-decaffeinate invokes eslint with --format json, which should always have a messages key in the output, but looks like that's not the case here.

There's a daily build that uses the latest bulk-decaffeinate against the latest eslint ( https://travis-ci.org/decaffeinate/decaffeinate-example-builder ) and it's not broken, so I imagine this may be something specific to your case. But regardless, bulk-decaffeinate should be better about how it handles the eslint output.

hefox commented 7 years ago

Looking into it further, for some reason it is not making the .js files

Situation: directory with .coffee, and auto created .map. and .js Deleted the .js's and ran convert on it The .coffee are moved to original and the original.js's are auto created but the converted .js's are not made, so eslint is run on the autocreated original.js and produce the above error situation.

Coworker ran it on same code base with same versions and didn't encounter this situation.

hefox commented 7 years ago

Solved via disabling the coffee compiler, duh. Feel free to close this (unless want to add additional error handling to catch 1) .js not being generated correctly 2) error in eslint json where message is not always set ).

tim-kos commented 6 years ago

I have this error, too. :/ It's super annoying to run into this after a ~30min conversion (large code base).

How exactly did you solve it?

Solved via disabling the coffee compiler, duh

What exactly did you do here? Thank you for your help!

alangpierce commented 6 years ago

@tim-kos does it always happen or just on a giant codebase? For a real production conversion, it's maybe best to do it incrementally anyway. At my work the most I converted at once was about 20,000 lines.

The code for the error is here: https://github.com/decaffeinate/bulk-decaffeinate/blob/master/src/modernize/runEslintFix.js

Something is going wrong with eslintStdout and eslintOutput. If you want, you can dig into it and see what those values are, that would help figure out why messages isn't in the ESLint output. Even if it's unclear what's going wrong, there certainly could be a better error message here, and I'd be happy to accept a pull request for any improvements here.

One thing that I remember is that ESLint sometimes gave more output than the default node buffer size, so I bumped the limit to 10MB of output. But maybe you have a giant file that has more than 10MB of ESLint errors after decaffeinate.

Solved via disabling the coffee compiler, duh

What exactly did you do here?

My impression was that they had the CoffeeScript compiler running in watch mode that was compiling .coffee to .js, and that was causing conflicts with the JS files being produced by decaffeinate.

tim-kos commented 6 years ago

Hey there,

Thanks for the hint. Btw, in the (dist) source I found

_context.t17 = config.skipEslintFix;

However, that config is not documented in --help. --skip-verify is however. You may want to add documentation for --skip-eslint-fix. Would have saved me some debugging. I am happy to apply the eslint fix calls myself afterwards on the converted code base. :)

tim-kos commented 6 years ago

It's also not in the commander options. I have manually added it and am now re-running the conversion.

The reason is, I want to get an idea of how big this job would be by doing a full conversion in a branch, and then running our testsuite (close to 100% coverage) on it. : ]

alangpierce commented 6 years ago

@tim-kos cool, hope that's a reasonable workaround for you.

I mostly added skipEslintFix to fix a build timeout in one of the CI builds. I'd recommend trying to get the built-in eslint stuff working, although of course you're welcome to use the option. It's a little smarter than just running eslint --fix on everything, it also looks at what remaining lint failures exist after autofixes and individually disables those lint rules via a comment at the top of the file. That way, you get a clean lint build for those files and can incrementally re-enable lint rules as part of code cleanup.

And yep, makes sense you'd want to try out decaffeinate on the whole thing for testing purposes, I've certainly done that plenty of times myself. I guess if it's just for testing then running eslint --fix yourself should work fine. (Or not running it at all, although I've seen a few bugs in eslint --fix, but they're definitely rare.)