Skip to content
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

Issues with prettier --parser option #150

Open
ItsHoff opened this issue Jan 24, 2023 · 12 comments
Open

Issues with prettier --parser option #150

ItsHoff opened this issue Jan 24, 2023 · 12 comments

Comments

@ItsHoff
Copy link
Contributor

ItsHoff commented Jan 24, 2023

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 #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.

@raxod502
Copy link
Member

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?

@ItsHoff
Copy link
Contributor Author

ItsHoff commented Jan 27, 2023

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.

@raxod502
Copy link
Member

raxod502 commented Feb 4, 2023

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.

@ItsHoff
Copy link
Contributor Author

ItsHoff commented Feb 6, 2023

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.

@raxod502
Copy link
Member

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.

@ItsHoff
Copy link
Contributor Author

ItsHoff commented Feb 26, 2023

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

@raxod502
Copy link
Member

raxod502 commented Mar 3, 2023

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.

@ItsHoff
Copy link
Contributor Author

ItsHoff commented Mar 3, 2023

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.

@raxod502
Copy link
Member

raxod502 commented Mar 4, 2023

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.

@danielpza
Copy link
Contributor

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

@raxod502
Copy link
Member

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.

@danielpza
Copy link
Contributor

danielpza commented Mar 30, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants