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: esbuildOutputFromId for symlinked root #10154

Merged
merged 1 commit into from Sep 18, 2022
Merged

Conversation

patak-dev
Copy link
Member

@patak-dev patak-dev commented Sep 17, 2022

Description

Fixes #9327

I think this should work around the issue. It looks like esbuild is returning a redundant path when the root was symlinked (maybe it is something to fix at esbuild?). @AlttiRi @vricosti would you help us test this on Windows? I don't know if this is only one of the issues we will experience with this setup though.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@AlttiRi
Copy link

AlttiRi commented Sep 17, 2022

After replacing

function esbuildOutputFromId(outputs, id, cacheDirOutputPath) {
    const flatId = flattenId(id) + '.js';
    return outputs[normalizePath$3(path$n.relative(process.cwd(), path$n.join(cacheDirOutputPath, flatId)))];
}

by

function esbuildOutputFromId(outputs, id, cacheDirOutputPath) {
    const cwd = process.cwd()
    const flatId = flattenId(id) + '.js';
    const normalizedOutputPath = normalizePath$3(
      path$n.relative(cwd, path$n.join(cacheDirOutputPath, flatId))
    )
    const output = outputs[normalizedOutputPath]
    if (output) {
      return output
    }
    for (const [key, value] of Object.entries(outputs)) {
      if (normalizePath$3(path$n.relative(cwd, key)) === normalizedOutputPath) {
        return value
      }
    }
}

in \node_modules\vite\dist\node\chunks\dep-cff57044.js

If I did the edit correctly.

There is no more the error in the terminal.

However, in this case there is

Uncaught TypeError: Failed to resolve module specifier "vue". Relative references must start with either "/", "./", or "../".

in DevTools console. The site is not working.

Running from the common folder (not sym-linked) still works ok.

@patak-dev patak-dev added the p3-minor-bug 🔨 An edge case that only affects very specific usage (priority) label Sep 18, 2022
@patak-dev
Copy link
Member Author

patak-dev commented Sep 18, 2022

Thanks for testing @AlttiRi! I think we could still merge this one (@bluwy, what do you think?), but I don't know if the use case is common enough to justify the complexity. If this is important to you or others, that would be to work on a PR and see how far we are.

bluwy
bluwy approved these changes Sep 18, 2022
@bluwy
Copy link
Member

bluwy commented Sep 18, 2022

Yeah I think this PR looks good to merge.

However, in this case there is

Uncaught TypeError: Failed to resolve module specifier "vue". Relative references must start with either "/", "./", or "../".

in DevTools console. The site is not working.

This might be a different warning that was initially hidden by the bug this PR is fixing. We could gradually make the fixes to see what's causing this new issue too.

@patak-dev patak-dev merged commit fc5310f into main Sep 18, 2022
18 checks passed
@patak-dev patak-dev deleted the fix/esbuild-output-from-id branch Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-minor-bug 🔨 An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails on running in a symbolic linked directory
3 participants