feat: cancellable scan during optimization

[patak-dev]

Closes #11243

Co-authored-by: dominikg

Description

This is a rework of #11243 using esbuild added support for canceling builds.

I don't see this as a fix for #9986. If it helps, it is on hiding a race condition we should still work out.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[benmccann]

This looks pretty good to me!

I don't see this as a fix for #9986. If it helps, it is on hiding a race condition we should still work out.

Yeah, you're right, I realized that as well. This is just for the scan phase. We'd need to do the same thing for the actual optimization phase which is where #9986 is occurring

[benmccann]

Is the return type right here? It looks like its returning { cancel, result } and not Promise<{ cancel, result }>

[patak-dev]

I was following @dominikg's PR here. The esbuildContext is only available after waiting for other async operations. But I agree that from a consumer POV, it is better to avoid the double promise, vitejs/vite@5ff8d8b (#12225) avoids this at the expense of a bit more complexity inside scanImports

[patak-dev]

I don't see this as a fix for #9986. If it helps, it is on hiding a race condition we should still work out.

Yeah, you're right, I realized that as well. This is just for the scan phase. We'd need to do the same thing for the actual optimization phase which is where #9986 is occurring

I'll check this in another PR. What I'm thinking though is that we could avoid naming all temporal folders in the same way. That is forcing us to be very careful about serializing optimizer runs. We can add a unique key per run, so it will be deps_temp_a3f4fa, and then #9986 is no longer possible.

[dominikg]

I don't see this as a fix for #9986. If it helps, it is on hiding a race condition we should still work out.

Yeah, you're right, I realized that as well. This is just for the scan phase. We'd need to do the same thing for the actual optimization phase which is where #9986 is occurring

I'll check this in another PR. What I'm thinking though is that we could avoid naming all temporal folders in the same way. That is forcing us to be very careful about serializing optimizer runs. We can add a unique key per run, so it will be deps_temp_a3f4fa, and then #9986 is no longer possible.

We would need a way to clean up residue though. Even today sometimes temporary vite config bundle files remain on disk if the process terminates at the wrong point in time. It would be worse when the same was true for optimizer output.

[benmccann]

To clean up, we could delete anything beginning with deps_temp_ when starting the server

[dominikg]

To clean up, we could delete anything beginning with deps_temp_ when starting the server

needs to ensure that there isn't another server currently having a go at it. Having 2 running on the same setup at the same time was something that came up lately
#12150

and with optimize on build and parallel builds it might apply there too.

[benmccann]

needs to ensure that there isn't another server currently having a go at it

one way to do it would be to check if the folder is more than 1 day old then it's safe to delete or something like that