Feature request: prettier.getFileInfo(path) API and CLI option

[kachkaev]

We were discussing Prettier plugin support in Atom with @robwise and faced the default parser problem.

Until Prettier stops parsing unsupported files as suggested in #2884, its editor extensions need to be fed with a whitelist of supported scopes. Without this list, Prettier would throw an exception when a user presses cmd+s for an unsupported file type or silently fail on real parse errors, which is also undesired. Because #2884 cannot be resolved until Prettier 2.0 and this might take a while, is there any room for a new API call like prettier.isFileSupported(file) and a corresponding CLI option? This feature could be used by the editor extensions before applying format on safe, and as a consequence, save a user from maintaining a whitelist of scopes.

How do you see the exterior of this feature look like? I can help with a PR if it has a potential to land to Prettier < 2.0.

[ikatyang]

What's the difference between prettier.getSupportInfo() and the proposed API?

[kachkaev]

@ikatyang prettier.getSupportInfo() returns a JSON with a list of supported extensions and it's the user's job to then match this list with the given file path. The new potential API would let us skip this second step and get the answer straight away. Perhaps, it could return the name of the parser and some other metadata for a provided file name.

Given that formally any file is supported until #2884 is closed, the result could also contain some flag indicating that a fallback JS parser is being used. In this case, an editor extension would not format on save. Additionally, the result could tell if the file is in .prettierignore.

Having said that, I'm open to alternative solutions that would help resolve prettier/prettier-atom#395 (comment) 😉

[suchipi]

What about getParserForFilePath(path: string): string and isFilePathIgnored(path: string): boolean?

[suchipi]

Also, getParserForFilePath could NOT return the JS parser as a fallback, even without #2884. Since it's a new API, this behavior would be a non-breaking change.

I guess that means its return value would be ?string, not string.

[kachkaev]

@suchipi the problem with two separate API calls is that they will require Prettier process spawning two times before an editor can make an attempt to prettify a given file. So how about getFileInfo(path: string): FileInfo where FileInfo is:

interface FileInfo {
  path: string;
  parser?: string;
  ignored: boolean;
}

or until 2.0:

interface FileInfo {
  path: string;
  parser: string;
  parserIsFallback: boolean;
  ignored: boolean;
}

In this case a Prettier editor extension could call prettier --file-info $PATH and then execute Format on save if the parser is present in the output and ignored is not true. The good thing about FileInfo is that we can stuff it with more data in future releases and it won't be necessary to create extra API methods and CLI options. WDYT?

[robwise]

This FileInfo interface seems like a good idea to me. I like that you've also put whether the file is ignored (currently prettier-atom has to do this via its own implementation).

Maybe this is stating the obvious, but we'd also need to make sure that parsers coming from plugins are properly picked up when returning the file info results.

[kachkaev]

we'd also need to make sure that parsers coming from plugins are properly picked up

That should not be a problem as long as the plugins are shown in prettier --support-info. Picking up plugins is currently not working in a global npm Prettier instance, but I hope a PR that fixes this gets through sooner or later. In any case, we can address that issue independently.

[suchipi]

Seems reasonable; that said, you might consider bundling prettier_d with the plugin to avoid spin-up costs.

Is the path property on a FileInfo just a pass-through from the input path?

[kachkaev]

Is the path property on a FileInfo just a pass-through from the input path?

Yep, that's just an absolute path of a file. It may be useful when someone calls prettier --file-info **/*.* and thus gets the info for multiple files. Actually, I'd return an array in any case, even if we're querying info for just one file.

As of prettier_d, it feels a bit too advanced for me at the moment. I'm not sure when to best restart the server while running the editor given that .prettierrc and .prettierignore, plugins, etc. can be changed at any time. Good to know about its existence though!

[kachkaev]

@suchipi will you be happy if I submit a PR that adds this API?

getFileInfo(filePatterns: string[]): FileInfo[]

// before 2.0
interface FileInfo {
  filename: string;
  parser: string;
  parserIsFallback: boolean;
  ignored: boolean;
}
// after 2.0
interface FileInfo {
  filename: string;
  parser?: string;
  ignored: boolean;
}

prettier --file-info filePattern1 [...filePatternN]

@robwise will this API solve the concerns you raised in prettier/prettier-atom#395 (comment)?

[suchipi]

Yup, looks good to me 👍

[kachkaev]

Hmm... Now I'm wondering: at what point should .prettierignore come into play if I'm running prettier --file-info **/*.*? Should I get the list of all files including those in .git and node_modules or should that list only contain files that will be prettified? In the latter case, what's the point of ignored flag in FileInfo if we'll be getting [] when calling prettier --file-info path/to/ignored/file?

[suchipi]

I think that if the pattern matches multiple files, it should omit ignored entries. If it matches only one file, include it even if it's ignored (and the ignored boolean will be present).

That way, **/* won't include a ton of ignored entries, but you can use eg ./package.json to find out if the current file is ignored.

[kachkaev]

That creates interface modality, which can be pretty dangerous. I'm now tending to think that getting rid of ignore property and always referring to .gitignore is a cleaner solution.

If we do output an object for a single file and a pre-filtered array for a glob, then what do we do a glob resolves to just one file? What about two file paths given as two arguments? The appropriate output format for these edge cases is not easy to guess unless we're 100% consistent.

I remember struggling with an issue in remarkjs, which was also caused by the interface modality for one and several files. Excluding modality in the behaviour of --file-info will help Prettier users avoid similar problems.

Updated spec:

getFileInfo(filePatterns: string[]): FileInfo[]

// before 2.0
interface FileInfo {
  filename: string;
  parser: string;
  parserIsFallback: boolean;
}
// after 2.0
interface FileInfo {
  filename: string;
  parser?: string;
}

prettier --file-info filePattern1 [...filePatternN]

prettier --file-info "**/*.*"
# FileInfo[] for files that are not excluded by prettierignore

prettier --file-info single/path.ext
# FileInfo[] of length 1 of the given file exists and is not in prettierginore
# [] otherwise

prettier --file-info some/path.ext other/path.ext
# array with 0, 1 or 2 members, depending on file existence and prettierignore 

prettier --file-info non/existing/path1 non/existing/path2
# []

[suchipi]

Sorry, I didn't mean that we would sometimes not return an array, I just meant that the array would sometimes be of length 1, and sometimes have more entries.

Aside from removing the ignored property, it looks like that's still what you're doing in your code block...?

[suchipi]

Oh, I see, you're always filtering out ignored files.

🤷‍♀️

Feels kinda 6-in-the-one, half-dozen-in-the-other to me.

[kachkaev]

Yep, I propose to always exclude non-existing and ignored files from FileInfo[] for API consistency. A consumer of the method will always only see the list of files that Prettier will attempt to format if it's ran with the same args but without --file-info. That looks like the cleanest possible API design for this case, but I'm open to other interface suggestions.

Once we've agreed, I'm happy to proceed with a PR – really want to help @robwise with his Atom package simplification!

[suchipi]

Sounds good to me

[robwise]

@robwise will this API solve the concerns you raised in prettier/prettier-atom#395 (comment)?

Yes, but just in case this wasn't clear, most plugins do not use prettier via the CLI, we use it programmatically so it needs to be a part of the API, not the CLI (or I guess you could do both).

Also, for the pre-2.0 version, is the idea that prettier-atom would check for fallbackParser and not try to format if this is true?

I'm now tending to think that getting rid of ignore property and always referring to .gitignore is a cleaner solution.

Referring to .gitgnore? What do you mean here? I think just returning null from getFileInfo to mean the file is prettierignored isn't totally intuitive. Why do we handle it this way for prettierignore but not for an unknown parser?

[kachkaev]

it needs to be a part of the API, not the CLI

Yeah that's my plan.

is the idea that prettier-atom would check for fallbackParser and not try to format if this is true?

Yep, you could do that. Editor plugin can have a checkbox, allowing people to either not format unknown files at all or format them with a fallback parser.

Referring to .gitgnore?

Sorry I meant .prettierignore here! 😅 I propose that getFileInfo never returns null, the result is always an array. You can interpret an empty array as null later in your code if you want by checking its length:

const fileInfos = prettier.getFileInfo([pathToFile]);
if (fileInfos.length /* check against prettierignore picked by the prettier instance (and file existence) */
    && !fileInfos[0].parserIsFallback /* optional check before 2.0 */) {
  formatBeforeSave();
}

I know this API may look a bit strange for a single file, but this is to open more opportunities when FileInfo needs to be obtained for multiple files/globs.

@robwise are you OK with this design? Shall I kick off the PR?

---

UPD: It should probably be getFileInfos and --file-infos to a better clarity. @suchipi OK?

[suchipi]

@kachkaev 👍

[robwise]

It's a bit unintuitive that a file being prettierignored means the file doesn't show up in the array, but a file not having an appropriate parser does show up.

It seems to me that it would be more consistent if we represented both cases consistently, such as using flags for both as originally suggested.

I know this API may look a bit strange for a single file, but this is to open more opportunities when FileInfo needs to be obtained for multiple files/globs.

This is breaking the YAGNI principle IMO—we're making the API for the thing we actually need (determining if we're able to format a given, single file) very strange in order to support something we're not even sure will ever be useful.

With this proposed array API, in order to determine what I want, I will have to check for 3 things and (somewhat unintuitvely), do so only for the first item of the array. This is going to seem very strange to those reading the code ("What about the other elements in the array?" or "Why is it called file info but is actually an array?"):

const fileInfo = getFileInfo(file);
if (fileInfo[0] && !fileInfo[0].parserIsFallback && fileInfo[0].parser) {
  format(file)
}

I suggest we solve the problem at hand; determining whether we should format a file or not. If we want to return an object instead of a simple boolean so we can add additional info later, that's fine, but let's still have a simple boolean flag like isFormattable or something so I can check one property. And I think we should only return an array if I pass a pattern (or leave that for a different function later on if and when we ever need that functionality).

That way I can just do:

if (getFileInfo(file).formattable) {
  format(file)
}

Oh that reminds me of something—resolveConfig is asynchronous so it also has a resolveConfig.sync. We should think about what we want to do in this case.

[kachkaev]

@robwise I take your argument about simplicity – indeed, let's start with just getFileInfo that would work for one file only and return an object, not an array.

I'll work on a PR this afternoon and will let know about the results.

[kachkaev]

➡️ #4341

[duailibe]

I've read this thread an the PR superficially, so excuse me if I say something wrong, but I tried to give the most attention I could.

@kachkaev and @robwise, with #2884, we'd only need an additional API for isPathIgnored, right? If the file is not ignored, always call Prettier on it. Am I missing something?

Now maybe the editor integration wants to be smart: when an user opens a file, check if a parser can be inferred and cache that information to prevent ever calling Prettier. Now a getParser(filepath) would make sense.

I think getFileInfo() is too broad and can be easily abused. For example, I don't think exists should be part of it.

Let me know what you think!

[kachkaev]

with #2884, we'd only need an additional API for isPathIgnored, right?

Yep, I guess so. The main reason for #4341 is an assumption that a fallback babel parser will be used at least till Prettier 2.0 for backwards compatibility. getParser() and isPathIgnored() sound cleaner to me when it comes to using Prettier methods, but I'm not sure that having two extra CLI options is a good thing. Do we want to bloat CLI API this much?

If getFileInfo() is not the ideal solution and it'll be quicker get default parser removed, I'm totally happy with an alternative approach. I'm just trying to get Prettier working correctly with plugins and not rely on hand-crafted lists of file types that current implementations of editors have to use 🙂 WDYT?

[duailibe]

@kachkaev

The main reason for #4341 is an assumption that a fallback babel parser will be used at least till Prettier 2.0 for backwards compatibility.

I don't think we need to wait for that. TBH the main reason we haven't done is nobody did the work yet. Otherwise, we'd have already reached out to understand better if that'd break anything important.

getParser() and isPathIgnored() sound cleaner to me when it comes to using Prettier methods, but I'm not sure that having two extra CLI options is a good thing

To clarify, getParser() wouldn't be necessary unless you wanted to "cache" that information. It's kind of an optimization. You can just always call Prettier on the file.

I'm just trying to get Prettier working correctly with plugins and not rely on hand-crafted lists of file types that current implementations of editors have to use

I'm on board 👍

[kachkaev]

Let's see if removing default parser can be fast-tracked.

To clarify, getParser() wouldn't be necessary unless you wanted to "cache" that information. It's kind of an optimization. You can just always call Prettier on the file.

Agree. But would not calling getParser() or getFileInfo() be considered a good practice? Imagine a huge csv file that I'm editing in VSCode or Atom and then hitting save. Do we want all these megabytes go via Prettier instead of just passing it a filename and getting I won't parse this in return? Remember, we can't black-list any extensions or discriminate by file size because of prettier plugins.

I'm on board 👍

👍 One more thing I'm trying to get through is #4192. It'd be great if you could give a hand there.

[duailibe]

@kachkaev Fair enough. 👍 I'm convinced this is a good api

(Sorry I deleted the previous comment as I noticed it didn't make any sense)

[kachkaev]

Not sure it will help. Here are the usual workflows in an editor as I see them:

1. make changes
2. run prettier manually

—

1. enable format on save
2. edit
3. press save and see prettier first formatting my text and then saving it to a fs

I’m not sure how avoid getFileInfo() or getParser() + isIgnored() here. We can’t hit a file system in the first case and we don’t want to send prettier large csvs or simply unsupported formats for a dummy run in the second case.

[kachkaev]

I'm really amazed to see this one and #4000 fixed! ❤️

~~~I could also cut a new unofficial version as I [once did earlier](https://github.com/prettier/prettier/issues/4000#issuecomment-376119478), but it'd be probably better if it was ‘official’. There is a past record of beta/RCs for prettier package, although these were not very frequent~~~

↑ Copied this to https://github.com/prettier/prettier/issues/4444#issuecomment-387873575