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

fix: plugin not working on windows #3

Merged
merged 7 commits into from
Sep 18, 2021

Conversation

userquin
Copy link
Member

@userquin userquin commented Sep 16, 2021

This PR fix the plugin not working on windows: the problem is that the browser requests the asset with <UNIT>:/ and so the browser (at least chrome) detects the request as a file request file:///<UNIT>:/.....

To solve the problem, we intercept the /__inspect_api/resolve request on the middleware and transform it to return this url /__resolve_windows/<UNIT>/.... when the server is running on windows and the id matches the following pattern /^([A-Z]+):\/(.*)/, where the <UNIT> is the first capturing group (maybe we can add also lower case a-z but I think it is always on upper case and add +, I don't remember the max number of units windows can mount).

The first problem is solved, that is, the browser will not fail when fetching the resolved asset:

// src/client/pages/index/module.vue::18
async function refetch() {
  /*
  ON WINDOWS NOW THE MIDDLEWARE WILL TRANSFORM THE RESOLVED ID
  FOR     ===>  /__inspect_api/resolve?id=<UNIT>:/...
  TO      ===>  /__resolve_windows/<UNIT>/....
  INSTEAD ===>  <UNIT>:/...
  */
  const { id: resolved } = await fetch(`/__inspect_api/resolve?id=${id.value}`).then(r => r.json())
  if (resolved) {
  // revaluate the module (if it's not initialized by the module graph)
    try { await fetch(resolved) } // ===> HERE THE BROWSER NOW WILL NOT FAIL ON WINDOWS <===
    catch (e) {}
  }
  await execute()
}

Then, we need to restore the original id on server when the id starts with /__resolve_windows/ for:

  1. plugin.transform, plugin.load and plugin.resolveId.
  2. for middleware on /__inspect_api, I don't know if it is necessary to do it on /module and /clear, since the client will send the orginal asset name on query request and /list will return the server names.

closes #2

@userquin userquin marked this pull request as ready for review September 16, 2021 20:37
@userquin
Copy link
Member Author

@antfu maybe some logic can be extracted to a common funcion on plugin.* functions...

@antfu antfu merged commit d86bd4b into antfu-collective:main Sep 18, 2021
@antfu
Copy link
Member

antfu commented Sep 18, 2021

This breaks the plugin on mac, not sure why yet, but sorry man I'd need to revert it for now

antfu added a commit that referenced this pull request Sep 18, 2021
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

Successfully merging this pull request may close these issues.

plugin doesn't work on Windows
2 participants