Unicode escape encoding in import paths (v3)

[gajus]

Rollup Version

v3.0.0-4

Operating System (or Browser)

macOS

Node Version (if applicable)

v18.7.0

Link To Reproduction

N/A

Expected Behaviour

dist/server/assets/_default.page.server.8dd7229c.js to be correctly referenced in dist/server/pageFiles.js.

const pageFilesLazyServer1 = {
  "/renderer/_default.page.server.tsx": () => import("./assets/_default.page.server.\uF7F9\uE4D30001\uE3CC\uF1FE.js")
};

Actual Behaviour

const pageFilesLazyServer1 = {
  "/renderer/_default.page.server.tsx": () => import("./assets/_default.page.server.8dd7229c.js")
};

Not clear where this encoding is coming from and whether this is a Rollup issue or vite-plugin-ssr issue (@brillout)

[gajus]

Looks like it is in other files too, e.g.

dist/client/entry-src/pages/opportunity/_slug/edit.page.client.afab3e2c.js
1:import{b0 as r,b1 as t}from"../../../../entry-renderer/_default.page.client.\uF7F9\uE4D30001\uE3CC\uF1FE.js";import"../../../../entry-entry-client-routing.\uF7F9\uE4D30003\uE3CC\uF1FE.js";export{r as Page,t as pageConcreteRequest};

[brillout]

@gajus These are generated by Vite, see https://github.com/vitejs/vite/blob/aceaefc19eaa05c76b8a7adec035a0e4b33694c6/packages/vite/src/node/plugins/importMetaGlob.ts and https://vitejs.dev/guide/features.html#glob-import. It's likely a Vite bug; I'd dig into importMetaGlob.ts.

[gajus]

In our very specific case, we theoretically should be able to work around this by removing the hash from the URL, i.e.

assetFileNames: 'asset-[name].js',
chunkFileNames: 'chunk-[name].js',
entryFileNames: 'entry-[name].js',

This is because we have a build ID as part of the URL.

However, the above configuration sends Vite into what appears to be an infinite loop, ... just stuck at:

vite v3.0.9 building for production...
transforming (96) dist/client/entry-src/pages/settings/work-preferences.page.client.js

Cleaning dist (oddly) allows the client build to pass (not clear why existing dist would have any effect on rollup in this case). However, since vite-plugin-ssr builds project twice, it then immediately gets stuck on dist/server.

vite v3.0.9 building SSR bundle for production...
transforming (17) src/pages/opportunity/:slug/edit.page.route.ts

Nothing useful in debug logs as far as I can tell.

Given that behaviour changes based on whether anything is already in dist, this feels like a rollup thing.

[lukastaegert]

The characters are hash placeholders created by Rollup. The problem is that for some reason, the characters have been escaped, which is why Rollup cannot replace them with the proper hash.

The question is if it is some Vite plugin escaping them or some code path in Rollup itself.

Edit: I am quite sure it is not something within Rollup's sources. This is the logic that Rollup uses to escape special characters in imports:
https://github.com/rollup/rollup/blob/release-3.0.0/src/utils/escapeId.ts#L5-L8
It appears that Vite is escaping more characters in their ids, and this is causing problems. I would hope this can be solved on Vite side, but I lack knowledge of Vite's sources as to what might be causing this. Any insights cc @patak-dev @antfu ?

[lukastaegert]

Given that behaviour changes based on whether anything is already in dist, this feels like a rollup thing.

Rollup 3 contains quite a few breaking changes and it is very unlikely that current Vite will "just work" with Rollup 3 without modifications. That being said, having stuff in dist should not change Rollup behaviour as Rollup does not read dist, it just blindly replaces files if names collide.

[patak-dev]

@gajus I think you should create a minimal reproduction, using only Vite (and not vite-plugin-ssr) if the bug is on Vite's side and send a bug report in Vite's repo. This also seems unrelated to Rollup to me. FWIW, I've never seen a report of issues with hash encoding in the Vite repo so far. I wonder if some of the plugins you are using are getting in the way here.

PS: @lukastaegert is the rollup v3 branch stable enough for us to start tracking it in Vite? We can create a rollup-v3 branch on our side to start working out issues in that case.

[gajus]

For what it is worth, we are only using vite-plugin-ssr.

[brillout]

Vite-plugin-ssr sets build.rollupOptions.output.chunkFileNames and build.rollupOptions.output.assetFileName at https://github.com/brillout/vite-plugin-ssr/blob/9947e6a4a38f2e93a98aa6720efe0abca3eb3d30/vite-plugin-ssr/node/plugin/plugins/distFileNames.ts.

[lukastaegert]

The file names are not the problem, the problem is likely some Vite plugin that implements a load or transform renderChunk hook that writes import statements that escape the imported ids instead of writing them directly (or better, just escaping line-breaks, which is what Rollup does, see https://github.com/rollup/rollup/blob/release-3.0.0/src/utils/escapeId.ts#L5-L8).

[lukastaegert]

@patak-dev

is the rollup v3 branch stable enough for us to start tracking it in Vite? We can create a rollup-v3 branch on our side to start working out issues in that case.

Definitely. There are only three changes left that I want to merge, one will affect watch mode (which is likely not relevant for you), the other Rollup CLI (which again is likely irrelevant for Vite), and the last affects interop, i.e. how Rollup finds the default export of external CommonJS/AMD dependencies. The last may have some relevance for Vite, but I would expect it is not fundamental.

The core changes, i.e. mostly the hashing, is in place, and that is what is causing the issues here and what needs to be tested. If our approach of using a placeholder with special unicode characters in the filename just does not work for some reason, we need to know soon and think about alternatives.

[patak-dev]

I've been able to reproduce the issue using the Vite playgrounds, so this is something we need to update on Vite core side. I'll try to get to the bottom of this next week, but I think changes to Vite are expected given the changes in Rollup 3 so I wouldn't worry about this issue being a blocker for the new approach.

[patak-dev]

@lukastaegert, the encoding issue was due to esbuild being in ascii-only mode by default. WIP support for Rollup 3 in this PR (fixing this issue by switching the esbuild charset to utf-8)

• feat: rollup 3 vitejs/vite#9870

There are only a few failing test cases in the PR, I'll take a look at them later, but this looks good to me. I would still want to see what other maintainers think of needing to use utf-8 here. Another possibility would be that Rollup also looks for the escaped characters when replacing the hash placeholders. I imagine this could trip other tools too.

[lukastaegert]

Thanks for taking a look!

Another possibility would be that Rollup also looks for the escaped characters when replacing the hash placeholders

The problem is that we need the placeholder to have the exact same byte length as the actual hash to not break sourcemaps in this steps and avoid recalculating them. So I was looking for some way to create placeholders that we can easily Regexp for with basically no risk of accidental matches.

We can probably do something close while restricting to non-escaped characters, but it also depends on what we would have available.

Alternatively, we use very long placeholders, calculate a diff sourcemap and postpone sourcemap folding until after this step, but I hoped we could avoid this.

[patak-dev]

We can probably do something close while restricting to non-escaped characters, but it also depends on what we would have available.

Is using something like \0{....}\0 an option? Given that \0 has a special meaning already in the Rollup ecosystem and that is a character that browsers can't work with, I don't think people are using it for their file names. And it should be rare enough to find this combo in user code (comments, etc). One benefit is that the placeholders names will be nicer when logging them

But I still think that your current choice could still work, so probably good to wait for more feedback.

[lukastaegert]

The problem is that we want to avoid reparsing the file, so we do a full text search for placeholder strings. In that context, \0 would be problematic as it is still not too uncommon. And I actually see it as a feature that it is a valid file name so it is possible to pass it to things like path.join for transformations (Update: Apparently path.join has no issues with \0). Also, even \0 would not necessarily be safe from encoding issues—how about \x00, \u0000? Then I would prefer to go for some magic ASCII character combination like $rO~...~lL$

[patak-dev]

That last one would affect sourcemaps as you explained though. I think the current special chars may work well, and actually force everyone to avoid messing with utf-8. In Vite, I was actually surprised esbuild was going into ASCII only. We would do the change to utf-8 even without Rollup 3 forcing us. If there is demand, we could support ASCII only by running a regex that decodes only these hash markers to utf-8 so Rollup can properly find them. But I don't know if it is something we should support.

[lukastaegert]

I could also make the markers configurable. They are only defined and used in a single file, it would not be too much effort. Then people can replace them with any ASCII sequence they deem safe.

[patak-dev]

That is also a possibility, you could wait until there is a real use case to do so though. But if you think there is a chance to add it in the future, maybe it is good to already expose the markers (even if they are constant) so if we add special handling for them we don't hardcode the characters.

[lukastaegert]

If you think it is worthwhile to expose them, then I would directly turn them into an option so they are exposed via renderStart. Otherwise, I would need to think about another mechanism that would still work IF I make them a per-output option later (it could also be per-build but really it is purely a rendering/chunking thing, so output option would be natural).

[patak-dev]

To be honest, I think we could try to release Vite without support for esbuild ASCII mode. Maybe you could add this once we are sure it is going to be needed? But any option that you see is a good fit for Rollup would work for us here.

[gajus]

Is there a resolution here?

[patak-dev]

@gajus see:

• [v3.0] Use ASCII characters for hash placeholders #4631

I think you should close this issue. After #4631, esbuild will no longer mess with the markers (that are now ASCII-only). Vite is not yet prepared to use Rollup 3 though, you may experience other issues but we should probably better discuss them in the Vite repo. We will be working to release Vite 4 once Rollup 3 is stable.