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

Pre-warm idea #81

Open
dabrahams opened this issue Mar 3, 2021 · 9 comments
Open

Pre-warm idea #81

dabrahams opened this issue Mar 3, 2021 · 9 comments
Assignees

Comments

@dabrahams
Copy link
Contributor

Just a thought: why not start pre-warming in the background on first activation and wait for it (if it hasn't completed already) on first save? That would seem to give the ideal user experience in most cases.

@jscheid
Copy link
Owner

jscheid commented Mar 3, 2021

Great idea, it seems so obvious that I suspect there might be a reason why I didn't do it that way, as I remember spending quite a bit of time on tweaking the experience around first use back when I started out. But it's also possible it simply didn't occur to me. Let me take a closer look at this sometime soon.

@jscheid jscheid self-assigned this Mar 3, 2021
@jscheid
Copy link
Owner

jscheid commented Mar 4, 2021

What I've found out so far revisiting my code:

  1. It turns out that this is actually how I had intended for it to work, except that there's a bug which renders it useless: the warm-up message is sent in a generator function, but because iter-next isn't called on the resulting iterator, the function -- although it will be invoked -- is never actually executed and therefore the warm-up message never sent.
  2. Even if it were to work correctly, the subprocess is only started when a relevant buffer is visited (or is already open at the time global-prettier-mode is activated) -- but in this case, the subprocess is (in the default configuration) queried immediately for Prettier settings, which means that any background warm-up activity won't have time to complete before the next request comes in, and so will effectively be blocking anyway.

Aside from fixing that bug, perhaps an improvement would be a setting that starts the process at load time rather than at first use. Perhaps a new setting eager for prettier-pre-warm, and it would probably be OK to enable that by default, as presumably people will only load (as opposed to install) the package if/when they intend to use it. What do you think?

@dabrahams
Copy link
Contributor Author

dabrahams commented Mar 4, 2021 via email

@jscheid
Copy link
Owner

jscheid commented Mar 4, 2021

It's a bug in this package. The iterator shouldn't be discarded here:

(prettier--request-iter

I'll fix that as part of implementing eager pre-warming.

@angrybacon
Copy link

Posting here as this is seems somewhat related: https://prettier.io/blog/2022/06/14/2.7.0.html

This release includes a new --cache CLI option. Enabling this option will use some attributes as cache keys and format files only if they have changed. This could dramatically improve CLI performance.

Can this be leveraged? I'll admit that I only briefly looked at the source out of curiosity but don't have a MR to offer.

(not that I have any performance issues at the moment)

@jscheid
Copy link
Owner

jscheid commented Jun 14, 2022

Can this be leveraged?

Ah, thanks for the heads up, however this package isn't using the Prettier CLI, it's using the API.

At any rate, the intended usage is to format on save, and Emacs doesn't save unchanged files (it will print (No changes need to be saved) if you try.) Therefore, something like this would be beneficial only if you run prettier-prettify manually without changing anything, or if you make a no-op change (e.g. insert a space and then delete it.) Those seem like corner-cases to me that aren't really worth optimizing for, would you agree?

@angrybacon
Copy link

Definitely, thanks for the clarification 👍

@dabrahams
Copy link
Contributor Author

I'll fix that as part of implementing eager pre-warming.

Did you ever implement the eager pre-warming? Should we close this issue?

@jscheid
Copy link
Owner

jscheid commented Jun 15, 2022

@dabrahams I've actually started working on it recently as part of a major overhaul which I think of as v2.0. I've been working on it on and off and it's coming along nicely, watch this space.

It's going to be a variation of what we had discussed before -- specifically, in the (default) case where we sync over settings from Prettier, such as indentation width, I think we should block on first edit by default rather than on save.

This should still give a smooth experience in the common case where you don't immediately start editing a buffer after it opens. You'll usually navigate inside the buffer first or do something else entirely, such as opening a second file.

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

No branches or pull requests

3 participants