Import paths with query parameters containing ../ cause path normalization to fail

[BobbieGoede]

Describe the bug

We're @nuxtjs/i18n running into an issue with the way paths are being normalized when the paths contain query parameters with (more than one) ../.

vite/packages/vite/src/node/utils.ts

Line 210 in df77991

return path.posix.normalize(isWindows ? slash(id) : id)

I understand that path.posix.normalize tries to make sense of the path containing ../, but in our case it breaks the way files are being resolved later on. I noticed that the rollup plugin @rollup/plugin-node-resolve had a similar issue and fixed it by splitting the query parameters before resolving a path.

This is what is happening:

> path.posix.normalize('@nuxt/i18n/__config__?target=./config.ts')
'@nuxt/i18n/__config__?target=./config.ts'
> path.posix.normalize('@nuxt/i18n/__config__?target=../config.ts')
'@nuxt/i18n/__config__?target=../config.ts'
> path.posix.normalize('@nuxt/i18n/__config__?target=../../config.ts')
'@nuxt/i18n/config.ts'
> path.posix.normalize('@nuxt/i18n/__config__?target=../../../config.ts')
'@nuxt/config.ts'

I would think losing the query parameters or it influencing the path inside normalizePath is not intentional, but if it is, then do you have suggestions as to how to work around this? I would be happy to open a PR to implement a similar fix to the one referenced.

Reproduction

https://stackblitz.com/edit/vitejs-vite-ejipxg?file=main.js

Steps to reproduce

Using imports with a query parameters containing more than one ../, as seen in the reproduction, import viteLogo from '/vite.svg?target=../../'; will fail to resolve.

System Info

Not relevant

Used Package Manager

npm

Logs

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[stackblitz]

Start a new pull request in StackBlitz Codeflow.

• vitejs-vite-ejipxg (Open reproduction)

[patak-dev]

I don't recall frameworks using a query param with a relative path before. For a while, we were (somewhat) supporting ? in user code. So this couldn't be implemented. In Vite 4.3, we clarified at least internally with the team that ? won't be supported so we could try to support this request now.

Touching normalizePath isn't easy as it is used everywhere. I added it tentatively to the Vite 5 milestone. We could do a simple split by ? as you propose there, or we may need to check in all the places where this is used and do the split before the call.

Could you tell us more about the use case though? Using relative paths after the ? param may trip other tools too. Maybe it would be better to avoid them. I imagine you could keep a paths map in your plugin and use an id for your target referring to it.

[BobbieGoede]

Could you tell us more about the use case though? Using relative paths after the ? param may trip other tools too. Maybe it would be better to avoid them. I imagine you could keep a paths map in your plugin and use an id for your target referring to it.

(@kazupon should be able to explain the use case better than I do as he implemented it.)
As I understand it we use paths starting with @nuxtjs/i18n/__config__ or @nuxtjs/i18n/__locale__ to serve different types of files, using the target parameter to retrieve these files relative to the Nuxt project using the @nuxtjs/i18n module.

Due to a new Nuxt feature (layers) these paths can be multiple levels up from the running project, which causes these parameters to break the path during normalization.

I'm not familiar enough yet with the way plugins for Vite work yet myself and ran into this trying to debug our issue, but I will look into your suggestion of keeping a paths map with IDs!

Touching normalizePath isn't easy as it is used everywhere. I added it tentatively to the Vite 5 milestone. We could do a simple split by ? as you propose there, or we may need to check in all the places where this is used and do the split before the call.

I noticed it being used a lot and I wouldn't want to introduce breaking changes but are there scenarios in which it is expected or preferred for normalizePath to also include the query parameters when normalizing paths?

[patak-dev]

Touching normalizePath isn't easy as it is used everywhere. I added it tentatively to the Vite 5 milestone. We could do a simple split by ? as you propose there, or we may need to check in all the places where this is used and do the split before the call.

I noticed it being used a lot and I wouldn't want to introduce breaking changes but are there scenarios in which it is expected or preferred for normalizePath to also include the query parameters when normalizing paths?

? is a valid identifier for a path but Vite already doesn't support it, so I think there shouldn't be a case. But as this function is exported and used in several community plugins we should be careful when changing it. It would be good to know if there are other cases where people are using paths in the query string to justify it.

[kazupon]

(@kazupon should be able to explain the use case better than I do as he implemented it.) As I understand it we use paths starting with @nuxtjs/i18n/__config__ or @nuxtjs/i18n/__locale__ to serve different types of files, using the target parameter to retrieve these files relative to the Nuxt project using the @nuxtjs/i18n module.

Due to a new Nuxt feature (layers) these paths can be multiple levels up from the running project, which causes these parameters to break the path during normalization.

@BobbieGoede
Thanks for the explanation. And thanks for open an issue to solve the nuxt i18n issue!
I’ve just found out this issue, maybe we can fix thiss issue, for without modifying the vite side.
(That is simply a matter of importing the target by relative paths)
I will work on a PoC to resolve the nuxt i18n issue, including refactoring the vite plugin.

I will add for your reference in the following, so background on using relative paths for queries in @nuxtjs/i18n/__config__ or @nuxtjs/i18n/__locale__.

The background is influenced by the locale messages used by nuxt i18n and the architecture of vue-i18n's handling of them used internally by nuxt i18n.
Locale messages is a dictionary of translation messages with a hierarchical structure like key-value used in translation or object in javascript, as shown below.

Key-value case:

{
  “hello”: “hello world!”
}

hierarchical structure case:

export default {
  nest: {
     bar: {
       buz: “…"
     }
  }
}

The nuxt i18n module supports the formats json, json5, yaml, js and ts for locale messags.
json, json5 and yaml are the locale messages as static resource, they are not changed locale messages defined in the file.
For js and ts, they are dynamically and comoposable locale messages as dynamic resources, because they can be programmatically processed. For example, they can be retrieved from a back-end DB via an API as follows

export default defineI18nLocale((locale) => {
  // for example, fetch locale messages from nuxt server
  return $fetch(`/api/${locale}`)
})

A bundler like vite can simply import a file with these locale messages and process it. For nuxt i18n, however, these locale messages must be precompiled into JavaScript in order to be processed.

You can see here for the reason for precompiling.
https://vue-i18n.intlify.dev/guide/advanced/optimization.html

For static resources such as json, the vite plugin can precompile the locale messages at bundle time. However, dynamic resources such as js are dynamic and cannot be pre-compiled at bundle time. So I used vite's virutal module to pre-compile dynamic resources bypassing @nuxtjs/i18n/__config__ to pre-compile them with nuxt i18n. pre-compile with nuxt i18n for dynamic resources.

After some development of nuxt i18n module, the vue-i18n compiler used inside nuxt i18n can now compile locale messages with JIT style.
So bypassing by the virutal module is might not necessary.

I need to try nuxt i18n module refactoring to see if it needs virutal module.
Please wait a little.

[dargmuesli]

The underlying issue has been resolved in @nuxtjs/i18n-edge 🥳 Thank you, @kazupon ❤️

[patak-dev]

Awesome! Thanks @kazupon for modifying @nuxjs/i18n!
@dargmuesli let's close this issue and the associated PR for now then. We can review changing normalizePath once we have a real issue on our hands again. I think it may be a better idea to avoid path-like values on the query param as we discussed.