prettier / prettier-atom

An atom package for the prettier formatter.
MIT License
756 stars 96 forks source link

CRITICAL : plugin breaks saving of all files #466

Closed awilkins closed 5 years ago

awilkins commented 5 years ago

Atom 1.33.0-beta2 Ubuntu 16.04 prettier-atom 0.56.0 Verified on 2 separate machines.

After installation, all file saves fail with the below stack trace silently emitted to the dev console (no UI popup, which is a separate issue ; no unexpected exception should pop the stack without a chance to report the problem).

Looks like code that decides whether to format based on file name. I wouldn't have expected this plugin to even be active for buffers that don't contain a scope that prettier can format (my failures were for markdown and terraform files). Would possibly be better if it decided to format based on scope decided by Atom.

Reproduce : change a file and try to save it. File is not saved. Blue pip remains on editor tab.

Workaround : disable plugin. Can now save files.

Uncaught (in promise) TypeError: Expected a function
    at Function.<anonymous> (/home/awilkins/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:48:423)
    at n (/home/awilkins/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:89)
    at Function.<anonymous> (/home/awilkins/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:63:257)
    at Function.result (/home/awilkins/.atom/packages/prettier-atom/node_modules/lodash/fp/_baseConvert.js:510:21)
    at Object.<anonymous> (/home/awilkins/.atom/packages/prettier-atom/dist/formatOnSave/shouldFormatOnSave.js:25:46)
    at Object.<anonymous> (/home/awilkins/.atom/packages/prettier-atom/dist/formatOnSave/shouldFormatOnSave.js:43:3)
    at Module.get_Module._compile (/usr/share/atom-beta/resources/app/static/<embedded>:11:146684)
    at Object.value [as .js] (/usr/share/atom-beta/resources/app/static/<embedded>:11:150231)
    at Module.load (module.js:561:32)
    at tryModuleLoad (module.js:504:12)
    at Function.Module._load (module.js:496:3)
    at Module.require (file:///usr/share/atom-beta/resources/app.asar/static/index.js:47:45)
    at require (/usr/share/atom-beta/resources/app/static/<embedded>:11:145974)
    at Object.<anonymous> (/home/awilkins/.atom/packages/prettier-atom/dist/formatOnSave/index.js:8:28)
    at Object.<anonymous> (/home/awilkins/.atom/packages/prettier-atom/dist/formatOnSave/index.js:17:3)
    at Module.get_Module._compile (/usr/share/atom-beta/resources/app/static/<embedded>:11:146684)
    at Object.value [as .js] (/usr/share/atom-beta/resources/app/static/<embedded>:11:150231)
    at Module.load (module.js:561:32)
    at tryModuleLoad (module.js:504:12)
    at Function.Module._load (module.js:496:3)
    at Module.require (file:///usr/share/atom-beta/resources/app.asar/static/index.js:47:45)
    at require (/usr/share/atom-beta/resources/app/static/<embedded>:11:145974)
    at /home/awilkins/.atom/packages/prettier-atom/dist/main.js:37:39
    at Generator.next (<anonymous>)
    at step (/home/awilkins/.atom/packages/prettier-atom/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:30)
    at /home/awilkins/.atom/packages/prettier-atom/node_modules/babel-runtime/helpers/asyncToGenerator.js:35:14
    at new Promise (<anonymous>)
    at new F (/home/awilkins/.atom/packages/prettier-atom/node_modules/core-js/library/modules/_export.js:36:28)
    at /home/awilkins/.atom/packages/prettier-atom/node_modules/babel-runtime/helpers/asyncToGenerator.js:14:12
    at lazyFormatOnSave (/home/awilkins/.atom/packages/prettier-atom/dist/main.js:42:17)
    at subscriptions.add.editor.getBuffer.onWillSave (/home/awilkins/.atom/packages/prettier-atom/dist/main.js:111:119)
    at Function.simpleDispatch (/usr/share/atom-beta/resources/app/static/<embedded>:11:1189237)
    at /usr/share/atom-beta/resources/app/static/<embedded>:11:1190862
    at Array.map (<anonymous>)
    at Emitter.emitAsync (/usr/share/atom-beta/resources/app/static/<embedded>:11:1190825)
    at TextBuffer.saveTo (/usr/share/atom-beta/resources/app/static/<embedded>:11:496894)
    at <anonymous>
robwise commented 5 years ago

Sorry for the late response, I was unplugged over the Thanksgiving break.

Why We're Using Prettier to Determine if File is Formattable

One of the problems we're having is that we can no longer determine whether to format based on Atom scopes because whether or not a file is formattable by Prettier might depend on a Prettier plugin that you installed that we'd have no way to know about. So if you wanted to use a plugin to format some new filetype, you wouldn't be able to do that if we were controlling which scopes were formattable.

Instead, a couple of months ago, we had Prettier create a function that allows us to check whether Prettier can properly infer a parser and then we use that to determine if the file is going to be formattable.

Besides this check, we also allow just passing a blacklist/whitelist scheme of globs as options to prettier-atom so people don't have to make project-specific configurations just to exclude files they commonly want to exclude, and that line of code seems to be where your error is actually originating.

Why We Swallow Errors

The whole swallowing of errors thing came about because many errors thrown by a Prettier plugin seem to not be handled by Prettier correctly and show up as undefined errors instead of the established error API for syntax errors.

This means I can't tell the difference from a simple syntax error thrown from a plugin and an actual show-stopping error. If you're using the plugin, then this can be extremely annoying to get sticky error popups every time you save with syntax errors, so it was requested that I disable them and only log them to console.

I agree that it's pretty sketchy and can lead to yak shaving. I'm not really sure what to do about it.

Fixing Your Error

I cannot reproduce on my current version of Atom. I'm wondering if the beta version of Atom you are using changed something in the API that we were relying on?

matrixik commented 5 years ago

I'm fine with errors as long as file is saved at the end. But error with this plugin block saving file. If error is encountered then don't block saving file even without formatting it.

samselikoff commented 5 years ago

I'm also getting an error which I think is related. With prettier-atom installed and enabled, if I

  1. Open any project in Atom
  2. Click Atom > Config...
  3. Try to save config.cson

then I get this error:

path should be a path.relative()d string, but got

image

If I disable prettier-atom I'm able to save the file

robwise commented 5 years ago

@samselikoff That seems to be an unrelated issue I think due to a change in the glob library we are using for checking blacklist/whitelist globs.

@matrixik Can you give instructions on how to reproduce? I'm not able to get this to happen so I can't really understand the problem. The way I wrote it, no error is ever supposed to block the saving process, so I'm not even sure what's going on.

matrixik commented 5 years ago

Hmmm, strange, can't reproduce it today. Sorry. The only problem now I have that it is not formatting html files, css and js is formatted on save fine. I don't see any errors in console (verbose level do not list even that file was formatted on save). Atom 1.32.2 and too many other plugins.

jwoLondon commented 5 years ago

Just to report I too am getting the same path should be path.relative() d string error as @samselikoff if I try to save any file (MacOSX 10.14.1, Atom 1.33.0). This has only appeared today since updating to latest version of prettier-atom.

While disabling format on save is a temporary workaround, it is a bit of a pain as it is easy to forget 'manual' formatting with the keyboard shortcut.

robwise commented 5 years ago

Just to report I too am getting the same path should be path.relative() d string error as @samselikoff if I try to save any file (MacOSX 10.14.1, Atom 1.33.0).

Hmm, very annoying, the underlying library sort of changed its API out from under me on this. I will work on a hotfix.

The only problem now I have that it is not formatting html files

This was fixed in #468 but I haven't released it yet.

MadLittleMods commented 5 years ago

@samselikoff @jwoLondon @robwise It looks like the path.relative() error is being tracked in https://github.com/prettier/prettier-atom/issues/473