fix: stop considering parent URLs as public file

[brillout]

Description

Currently, when the URL starts with /.../node_modules/ (e.g. /../node_modules/some-library/src/someFile.js) then checkPublicFile() wrongfully considers the file to be a public file.

This bug happens because path.join(publicDir, '/../node_modules/some-library/src/someFile.js') resolves to the actual file someFile.js which makes checkPublicFile() wrongfully believe that it's a public file.

Additional context

This sometimes happens for frameworks built on top of vite-plugin-ssr.

Reproduction: https://github.com/brillout/vite-reprod-11145.

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[bluwy]

I think it'll be great to have tests that can reproduce this. I'm not sure how this would happen in Vite currently, and maybe it's better to fix the root problem.

[brillout]

I think it'll be great to have tests that can reproduce this. I'm not sure how this would happen in Vite currently, and maybe it's better to fix the root problem.

👍 Makes sense. I will try to reproduce this as a test.

[brillout]

Just had a closer look.

I'm not sure how this would happen in Vite currently

It happens when you have this:

import.meta.glob('/../path/to/**/*.js')

Which is a valid pattern:

vite/packages/vite/src/node/__tests__/plugins/importGlob/fixture.test.ts

Line 51 in 388adf5

"import.meta.glob(['/../fixture-b/*.ts'])",

I think it'll be great to have tests that can reproduce this

I think it requires a full-fledged playground (because importAnalysisPlugin() requires a publicDir and a dev server). So I'm not sure if it's worth it to have an entire new playground just to test this edge case?

[bluwy]

That's quite a strange pattern to support. I'd ping Anthony but he's on vacation now. I'm not sure if we want to correctly support it though, and using relative ../../ instead would be safer. 🤔

[brillout]

That's quite a strange pattern to support. I'd ping Anthony but he's on vacation now. I'm not sure if we want to correctly support it though, and using relative ../../ instead would be safer. 🤔

Virtual modules cannot use ../../, so the only option is /../../. (Since /../../ is relative to config.root, whereas ../../ is relative to __dirname but __dirname doens't exist in virutal modules.)

Hydrogen introduced the practice of globbing inside node_modules/some-dependency/ which VPS also does.

When using pnpm, the file paths ends up being /../node_modules/some-library/some-file.js.

[patak-dev]

It looks like these paths are indeed valid. Interesting that this is the only place where things break though. If vite-ecosystem-ci passes, this LGTM.

[patak-dev]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI: Open

suite	result
astro	✅ success
histoire	✅ success
iles	✅ success
ladle	✅ success
laravel	✅ success
marko	✅ success
nuxt-framework	✅ success
previewjs	✅ success
rakkas	✅ success
sveltekit	✅ success
vite-plugin-ssr	✅ success
vite-plugin-react	✅ success
vite-plugin-react-swc	✅ success
vite-plugin-svelte	✅ success
vite-plugin-vue	✅ success
vite-setup-catalogue	✅ success
vitepress	✅ success
vitest	✅ success
windicss	✅ success