apheleia icon indicating copy to clipboard operation
apheleia copied to clipboard

Issues with prettier --parser option

Open ItsHoff opened this issue 2 years ago • 12 comments

Hey,

I noticed two issues caused by the explicit --parser option in prettier:

  • json files fail to parse when json-mode is not installed, since they open in js-mode by default.
  • Prettier has multiple json parsers (https://prettier.io/docs/en/options.html#parser) which differ slightly. In my case, prettier check was failing because apheleia was using --parser=json while the parser chosen by prettier was json-stringify.

I see that the option was added due to https://github.com/radian-software/apheleia/issues/103, so I'm not sure what the correct solution should be here. The issue is fairly simple to work around by updating apheleia-formatters.

ItsHoff avatar Jan 24 '23 08:01 ItsHoff

Hmm, well, there is no accounting for the user needing to use a specific parser for the json files in their project. That will have to be up to user configuration (although we could make it easier to add project-local configuration, if it's awkward). Is the choice of json-stringify a particular configuration in your project, or is that the default for your distribution of Prettier?

The js-mode issue is more troubling. I am not sure an elegant way that we could resolve that issue, since Emacs is saying pretty clearly that the file should be considered JavaScript code. I suppose we could add an entry to apheleia-mode-alist by default that maps "\\.json\\'" to prettier-json, what do you think?

raxod502 avatar Jan 27 '23 03:01 raxod502

Hmm, well, there is no accounting for the user needing to use a specific parser for the json files in their project. That will have to be up to user configuration (although we could make it easier to add project-local configuration, if it's awkward). Is the choice of json-stringify a particular configuration in your project, or is that the default for your distribution of Prettier?

From what I can see json-stringify is used for package.json and otherwise json is used. If I rename a package.json then the inferred parser will be json. (FYI, you can check the inferred parser with prettier --file-info.)

The js-mode issue is more troubling. I am not sure an elegant way that we could resolve that issue, since Emacs is saying pretty clearly that the file should be considered JavaScript code. I suppose we could add an entry to apheleia-mode-alist by default that maps "\\.json\\'" to prettier-json, what do you think?

Personally, I would let remove the explicit --parser option and let prettier decide the correct parser. It is possible to define the parsers in prettier config for files that prettier cannot infer automatically, and otherwise override the inferred parser. For example,

        "overrides": [
            {
                "files": [
                    ".eslintrc"
                ],
                "options": {
                    "parser": "json"
                }
            }
        ]

In my opinion, auto formatting should produce output that passes the format check, and explicitly setting the parser will break this in some scenarios.

The issue with the "let prettier decide" approach is when you want to use prettier auto formatting for a file, for which prettier cannot infer the parser, and which is not part of a project that uses prettier. So, maybe there should be an option to request an explicit parser when needed.

ItsHoff avatar Jan 27 '23 09:01 ItsHoff

Blah. Your way is how I had it originally, actually, but then people complained it didn't work for buffers that had a major mode yet no file associated with them. I'm not sure what the best way to accommodate both use cases is.

raxod502 avatar Feb 04 '23 00:02 raxod502

Yeah, unfortunately these requirements partially conflict with each other. To summarize the situation:

Prettier with --parser:

  • Works until the edited file belongs to a project that uses prettier and either the inferred parser is different from the explicit parser or the parser has been overriden in the prettier configuration (seems that the available parsers contain multiple options for js, ts and json).
  • When this fails, either prettier throws an error if the file cannot be parsed at all (likely in the case of wrong major mode like json file in js-mode), or in the worse case prettier formats the file using the wrong parser which can cause the file to fail a prettier check.

Prettier without --parser:

  • Works until prettier is unable to infer the parser for the file and the project does not contain a prettier configuration where the parser could be defined.
  • When this fails, prettier throws an error that it is not able to infer the parser.

After thinking about this some more, I am doubtful that an user option could solve the issue since the issue is less about user preference and more about the project in which apheleia is used. Personally, I would rather prioritize prettier formatting working in projects that use prettier rather than formatting working on files that do not belong in a project that uses prettier, and get rid of the --parser option. (Of course, I am slightly biased here.) Also, prettier should be able to infer the parser as long as the file suffix is correct. One workaround for the non-inferred files that comes to mind would be to check the inferred parser with prettier --file-info and only set the major mode based --parser if the inferred parser is null, or alternatively rerun prettier with --parser when prettier throws an error about uninferred parser.

ItsHoff avatar Feb 06 '23 13:02 ItsHoff

How about... creating a shell script that attempts to run Prettier with no explicit parser on the provided file, and if that fails, runs it with a default parser based on the filename? Then we can define an Apheleia formatter that uses that shell script, which would work in both cases.

I could make it so that commands get to know the buffer major mode, passed in via major mode.

raxod502 avatar Feb 25 '23 19:02 raxod502

Yeah, that should work. Though, I am bit worried how that would work on Windows.

ItsHoff avatar Feb 26 '23 15:02 ItsHoff

Generally speaking, I don't provide official support for Windows because I am unable to test on that platform, although contributors are welcome to submit patches to restore functionality that may break.

raxod502 avatar Mar 03 '23 01:03 raxod502

I'm one of the Windows users. I'm happy to report that I have not run into any Windows specific issues, and I'd prefer to avoid knowingly breaking Windows functionality. Though in the case of a shell script I'm guessing it should be enough to have sh present in the PATH, and if that is the case I have no objections with it. I find that these basic Unix tools are commonly required, and easily available, for example, via Git for Windows.

ItsHoff avatar Mar 03 '23 14:03 ItsHoff

Ok, that should be fine. Using only POSIX standard features in shell scripts is a best practice anyway, since some UNIX distributions use shells other than bash by default.

raxod502 avatar Mar 04 '23 01:03 raxod502

Is there a way to change the parser for specific files? say package.json? Is there a workaround to this issue?

danielpza avatar Mar 29 '23 20:03 danielpza

Sure, you can configure your Emacs to set the buffer-local variable apheleia-formatter to any configured formatter, or one you've registered yourself, in any desired sub-set of files or following whatever logic you prefer.

raxod502 avatar Mar 29 '23 21:03 raxod502

After some more research I was able to figure it out, I'll leave my workaround here in case someone finds it useful

  (add-hook 'json-ts-mode-hook 'my/set-package-json-apheleia-formatter)

  (defun my/set-package-json-apheleia-formatter ()
    "Set the apheleia formatter to json-stringnify for package.json file."
    (when (equal (file-name-nondirectory (buffer-file-name)) "package.json")
      (setq apheleia-formatter 'prettier-json-stringify)))

Also had to add a new prettier parser prettier-json-stringify

With this now apheleia correctly produces output that will pass prettier --check for package.json

danielpza avatar Mar 30 '23 15:03 danielpza