prettier / prettier-atom

An atom package for the prettier formatter.
MIT License
754 stars 95 forks source link

Watch .prettierrc for changes and reload without cache if so #270

Open benoitgrelard opened 7 years ago

benoitgrelard commented 7 years ago

If I change any configuration in my .prettierrc file after atom having been opened, the changes don't seem to be applied.

Is this intended? It seems the config is cached when atom launches and then it uses this one the whole time the editor is opened, meaning if I make a change to the config I need to restart atom.

It wasn't so intuitive to me initially and I would have thought it would lookup the .prettierrc config everytime it formats to make sure it's using the right config.

robwise commented 7 years ago

Well, it's intended in the sense that prettier has written their tool to do this for speed purposes. I believe I can pass a special option to force it not to cache, but I wasn't sure if this was a good idea or not.

darahak commented 7 years ago

I think we should still enforce caching most of the time, but watch the config file and use clearConfigCache when it changes.

benoitgrelard commented 7 years ago

@darahak That sounds like a great solution!

bradennapier commented 7 years ago

I have relaoded atom about 95 times now just trying to get print width right... desperately need this since printWidth is ignored in the actual config of the plugin

robwise commented 7 years ago

@bradennapier What do you mean the print width is ignored?

SavePointSam commented 6 years ago

While trying to tackle everything going on in #289, I wanted to start with this bug first.

There are a couple of things we will need to tackle in order to get this fixed properly. Firstly, we need to determine where we draw the line in terms of watching the file system for changes to find new/changed configurations. I would argue that we should only support watching files that exist in the current active project root. Allowing us to utilize Project#onDidChangeFiles. This will ultimately be more performant than trying to scan through any file system change provided via PathWatcher.

Granted, we would only use those when we are unable to find a configuration, or the previously loaded configuration has been lost. When we do have a configuration that was located, we can simply bind a watch on that file directly. However! There is a hierarchy to how Prettier says configs are loaded and used. So, even if we do have a loaded config, I would suggest that we watch the current project directory for any changes that may override the currently loaded configuration.

Ultimately, we would want to account for these use cases:

All of this is well and good. But, of course, we run into a whole new problem. Prettier doesn't expose the file path of the chosen configuration making it impossible for us to know which file to watch for changes. On the bright side, the logic for finding a configuration is fairly simple and uses a third-party solution. So, we could bake in our own configuration gathering system as comiconfig exposes the file path in it's result.filepath. Or, we could ask Prettier to expose the located file path.

Let me know what you think @robwise, I'd love to solve this problem soon. However, there are some pretty big decisions we need to make on how we tackle this.

robwise commented 6 years ago

@SavePointSam, I like the concept of using Project#onDidChangeFiles, but doing it always. This is similar to how React approaches DOM event listeners. They just listen to every event fired and then filter out the ones they don't need to care about.

We could do a similar approach where we listen for any file changes whatsoever, and then just do something ghetto like regex the filename for .prettierrc (or whatever) and then if it matches, we set some sort of boolean variable like refreshCache to true. Then we just make sure to check this variable when formatting so we know whether to tell prettier to break cache or not, and reset it back to false after formatting is successful.

darahak commented 6 years ago

Prettier doesn't expose the file path of the chosen configuration making it impossible for us to know which file to watch for changes.

@SavePointSam FWIW I opened https://github.com/prettier/prettier/issues/3451 to have the ability to find the config file for any given source. The function already exists, it's just not exposed. I guess most of the work would be on test coverage.

SavePointSam commented 6 years ago

Very true. I was actually tempted to just post a PR to expose it for us myself. Haha. Thanks for doing that! It’s certainly a better solution to have them expose it for us in the case they decide to change how they search for configuration in the future. 👍

SavePointSam commented 6 years ago

@robwise I was also thinking about the same thing. However, that will only solve for updates that exist inside the project directory. It’s certainly how I would handle the use case of ‘no configuration > watch the project directory’ and ‘external configuration > watch for override’.

I think I’ll take a look at exposing Prettier’s matches filepath and go from there 👍

odiak commented 6 years ago

How about adding command: "Prettier: Reload Config"? It looks easy to implement and better than nothing.

refactorized commented 6 years ago

How about adding command: "Prettier: Reload Config"? It looks easy to implement and better than nothing.

Yes please - and also can this be better called out in docs / the settings page - I have blown an hour trying to figure out what I was doing wrong config-wise, but it was just that I needed to restart atom.

not to down on prettier, or this plugin, they are great.

skythyx commented 3 years ago

hi sir, i got same issue. this is hard, always restart atom-editor after edit config-file :(

my config-file location "~/.atom/.prettierrc.json"