Skip to content

fix: tentative fix for absolute paths on windows#324

Open
mrginglymus wants to merge 1 commit into
unjs:mainfrom
mrginglymus:windows-abs
Open

fix: tentative fix for absolute paths on windows#324
mrginglymus wants to merge 1 commit into
unjs:mainfrom
mrginglymus:windows-abs

Conversation

@mrginglymus
Copy link
Copy Markdown

Given that upstream doesn't want to address this directly (wooorm/import-meta-resolve#31) and downstream is now blocked on this (storybookjs/storybook#32364) here's a tentative workaround for

@43081j
Copy link
Copy Markdown
Member

43081j commented Sep 3, 2025

let's wait for @pi0 but i wonder if we should be fixing this at the source instead of the call site

mlly/src/utils.ts

Lines 14 to 19 in e9faaf8

export function fileURLToPath(id: string | URL): string {
if (typeof id === "string" && !id.startsWith("file://")) {
return normalizeSlash(id);
}
return normalizeSlash(_fileURLToPath(id));
}

i feel like maybe this logic should be switcheroo'd?

export function fileURLToPath(id: string | URL): string {
  const idString = typeof id === 'string' ? id : id.href;
  if (id.startsWith("file://")) {
    return normalizeSlash(_fileURLToPath(id)); // pass the original `URL | string` since the built in fileURLToPath accepts both
  }
  return normalizeSlash(idString);
}

@pi0
Copy link
Copy Markdown
Member

pi0 commented Sep 3, 2025

Thanks for the PR. I haven't read it through but unlikely we make risky changes in mlly resolve utils anymore as should be deprecated.

I suggest migrating to exsolve and report any windows related issues (i think should be less/no issues with exsolve)

mrginglymus added a commit to mrginglymus/storybook that referenced this pull request Sep 3, 2025
mlly resolve utils are deprecated
(unjs/mlly#324 (comment)), and it
is recommended to use exsolve instead.

The old strip-abs-node-modules-path utility is reintroduced as there is
no equivalent in exsolve, and the mlly implementation was more or less
the same anyway (except using regex instead of split)
mrginglymus added a commit to mrginglymus/storybook that referenced this pull request Sep 3, 2025
mlly resolve utils are deprecated
(unjs/mlly#324 (comment)), and it
is recommended to use exsolve instead.

The old strip-abs-node-modules-path utility is reintroduced as there is
no equivalent in exsolve, and the mlly implementation was more or less
the same anyway (except using regex instead of split)
mrginglymus added a commit to mrginglymus/storybook that referenced this pull request Sep 3, 2025
mlly resolve utils are deprecated
(unjs/mlly#324 (comment)), and it
is recommended to use exsolve instead.

The old strip-abs-node-modules-path utility is reintroduced as there is
no equivalent in exsolve, and the mlly implementation was more or less
the same anyway (except using regex instead of split)
@mrginglymus
Copy link
Copy Markdown
Author

I've raised a PR to migrate to exsolve; however, at this late stage in the release cycle they may be unwilling to switch implementations.

@pi0
Copy link
Copy Markdown
Member

pi0 commented Sep 3, 2025

Added two comments on storybookjs/storybook#32383

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.

mlly throws [error] The URL must be of scheme file when called with absolute path on windows

3 participants