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

Expose pendingRequests #6011

Closed
4 tasks done
KaelWD opened this issue Dec 8, 2021 · 12 comments
Closed
4 tasks done

Expose pendingRequests #6011

KaelWD opened this issue Dec 8, 2021 · 12 comments

Comments

@KaelWD
Copy link
Contributor

KaelWD commented Dec 8, 2021

Clear and concise description of the problem

I'm writing a plugin that collects a list of all the imported sass files and writes them to a single file instead. I can't write them one by one because sass throws "variable not defined", so the transform request for the combined file has to be stalled until no other modules are waiting to be resolved.

Suggested solution

Currently I'm using server._pendingRequests which is exactly what I was looking for, but it is a private property so really needs to be documented and exposed in a way that would be compatible with rollup. I proposed using this.pendingRequests in rollup/rollup#4294.

Alternative

No response

Additional context

https://github.com/vuetifyjs/vuetify-loader/blob/274ce9ced8da65107b7544f9cdb2d82d463be313/packages/vite-plugin/src/stylesPlugin.ts

vuetifyjs/vuetify-loader#225
rollup/rollup#4294

Validations

@sodatea
Copy link
Member

sodatea commented Dec 9, 2021

Some background:

So now I'm using it in vite-jest too.
Seems worth exposing to me.
We'll discuss it at the next team meeting.

@patak-dev
Copy link
Member

Answer from rollup side: rollup/rollup#4294 (comment)

While you do not have this exact API, you can already do something similar with a combination of this.load and this.getModuleIds.
The id of a module is added to this.getModuleIds() before its load hook is even run, so all "pending" modules can be found here. When you have the id, you can await this.load(id) to wait until it is loaded and get its module info.

@KaelWD does getModuleIds() work in Vite also? Or should we rename this issue to implement/fix it?

@KaelWD
Copy link
Contributor Author

KaelWD commented Dec 11, 2021

server.moduleGraph looks similar, but there's no this.load() equivalent afaik.

@KaelWD
Copy link
Contributor Author

KaelWD commented Dec 11, 2021

I think I managed the same result without _pendingRequests. This seems like it might have some weird edge cases though, I already ran into stuff like /@id/plugin-vue:export-helper which always has null transformResult.

       return (await Promise.all(
-        Array.from(server._pendingRequests, ([k, v]) => {
-          const module = server.moduleGraph.urlToModuleMap.get(k)
-          return module?.id && !blockingModules.has(module.id)
-            ? v.then(() => module.id)
+        Array.from(server.moduleGraph.urlToModuleMap.entries(), ([k, v]) => {
+          return !k.startsWith('/@id/') && v.transformResult == null && !blockingModules.has(v.id!)
+            ? server.transformRequest(k).then(() => module.id)
             : null
         }).filter(v => v != null)

vuetifyjs/vuetify-loader@f78823d

@KaelWD
Copy link
Contributor Author

KaelWD commented Dec 11, 2021

This might have the same problem as rollup/rollup#4296

Screenshot_20211211_233639

@bluwy
Copy link
Member

bluwy commented Mar 31, 2022

@KaelWD I'm wondering what's the status of the feature request now. Vite 2.9 had made changes to server._pendingRequests to fix caching and race conditions. Does it still work today, or is there already a different solution?

@bluwy
Copy link
Member

bluwy commented Jun 26, 2022

FWIW the signature for server.pendingRequests has changed in Vite 3, and I'm not sure we can guarantee the API after Vite 3 to expose it. From this issue, the scenario looks a bit like how Vite detects deps for prebundling at the moment. IINM it uses a timeout and once there's no detected files over a set period of time, it bundles them up.

@KaelWD
Copy link
Contributor Author

KaelWD commented Jun 26, 2022

Does it still work today?

Not sure, I've been using the urlToModuleMap + server.transformRequest solution since december: https://github.com/vuetifyjs/vuetify-loader/blob/ac5af823f1bfd8bc79dc3eb353eed64adef34421/packages/vite-plugin/src/stylesPlugin.ts#L44-L56

I haven't tried vite 3 yet to see if this still works, having both vite and the plugin waiting for files sounds like it could cause some problems.

@KaelWD
Copy link
Contributor Author

KaelWD commented Jul 11, 2022

2.8.6 had the above problem (server.transformRequest() resolving too early)
2.9.0+ fixed that but has some requests with transformResult == null that never resolve, but _pendingRequests is empty
3.0.0-beta.9 has the same as 2.9.0 but they're also in _pendingRequests

@KaelWD
Copy link
Contributor Author

KaelWD commented Aug 26, 2022

I still don't have a stable version of this, everything I've tried eventually gets stuck on random files. The rollup version on the other hand is pretty simple and works great:

const modules = Array.from(context.getModuleIds())
  .filter(id => {
    return !blockingModules.has(id) && // Ignore the current file
      !/\w\.(s[ac]|c)ss/.test(id) // Ignore stylesheets
  })
  .map(id => context.getModuleInfo(id)!)
  .filter(module => module.code == null) // Ignore already loaded modules

pendingModules = modules.map(module => module.id)
if (!pendingModules.length) return 0

const promises = modules.map(module => context.load(module))
await Promise.all(promises)

vuetifyjs/vuetify-loader#263

@bluwy
Copy link
Member

bluwy commented Aug 28, 2022

I actually had to this something like this for my plugin, and thanks to your issue and code @KaelWD I implemented something similar like so:

// wait for all modules loaded before getting the entry ids
const seen = new Set()
let modulesToWait = []
do {
  modulesToWait = []
  for (const id of this.getModuleIds()) {
    if (seen.has(id)) continue
    seen.add(id)
    if (id.startsWith('\0')) continue
    const info = this.getModuleInfo(id)
    if (info?.isExternal) continue
    modulesToWait.push(this.load({ id }).catch(() => {}))
  }
  // TODO: timeout if too long
  await Promise.all(modulesToWait)
} while (modulesToWait.length > 0)

Besides the TODO, and that I've only tested it on a small project, it seems to be working well. I suppose this would work for Rollup too and I hope we can find a trick that works in both without Vite exposing pendingRequests.

@bluwy
Copy link
Member

bluwy commented Dec 30, 2022

I think the trick above should be good for now. Since we can't guarantee the stability of pendingRequests and it's not exactly needed, I'll go ahead and close this.

@bluwy bluwy closed this as not planned Won't fix, can't repro, duplicate, stale Dec 30, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jan 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants