fix: handle path with dollar signs

[merceyz]

What's the problem this PR addresses?

Vite doesn't work when the path it is in contains dollar signs

Fixes #1423
Ref https://twitter.com/youyuxi/status/1354094331876016134

How did you fix it?

Use the callback version of .replace.

More

This should be handled correctly in all places that uses .replace (for example

vite/packages/vite/src/node/plugins/clientInjections.ts

Lines 43 to 51 in 1e67d66

	.replace(`__MODE__`, JSON.stringify(config.mode))
	.replace(`__BASE__`, JSON.stringify(config.base))
	.replace(`__ROOT__`, JSON.stringify(config.root))
	.replace(`__DEFINES__`, JSON.stringify(config.define || {}))
	.replace(`__HMR_PROTOCOL__`, JSON.stringify(protocol))
	.replace(`__HMR_HOSTNAME__`, JSON.stringify(host))
	.replace(`__HMR_PORT__`, JSON.stringify(port))
	.replace(`__HMR_TIMEOUT__`, JSON.stringify(timeout))
	.replace(`__HMR_ENABLE_OVERLAY__`, JSON.stringify(overlay))

) but this serves more as a pointer than a full fix in the entire codebase

[yyx990803]

I really doubt this makes any difference at all. I created a project with $$ in its path, and also tried a package with $$ in its name, both are working fine (as long as no Yarn 2 is involved).

[yyx990803]

The replacement value cannot be a function. It also has nothing to do with native .replace calls. I have no idea what this fix is attempting to address.

[arcanis]

I really doubt this makes any difference at all. I created a project with $$ in its path, and also tried a package with $$ in its name, both are working fine (as long as no Yarn 2 is involved).

I think you're incorrect ... I literally just followed your website's getting started:

yarn create @vitejs/app
cd vite-project
npm install
npm run dev

I see "Hello Vite!". I then run (note the quotes around $$):

cd ..
mv vite-project '$$vite-project'
cd '$$vite-project'
npm run dev

Navigating to localhost yields an empty page and a 404 error. Just to double-check I then do a last test:

cd ..
cp -rf '$$vite-project' foo
cd foo
npm run dev

The "Hello Vue!" appears again. In other word, the $$ caused a 404 to appear.

[arcanis]

The replacement value cannot be a function. It also has nothing to do with native .replace calls. I have no idea what this fix is attempting to address.

The replace function replaces $1, $2, etc by their equivalent in the source regex. One little known feature is that it also replaces $$ by $ (basically, it escapes $ with itself).

The diff @merceyz's nicely made aims to prevent this behavior, which causes the breakages with $$ in the path (which I assume you may have not seen if you forgot to escape '$$' in your own shell, which gets translated as the current pid). By making the replacement a function (rather than a literal), the JS engine won't try anymore to interpret whatever you return, and will instead use it at face value - which is what you want.

Honestly, even if you don't repro the problem (and I think you should be able to fairly easily), it's easy to see what kind of problems the current code is susceptible to.

[merceyz]

The replacement value cannot be a function

It's passed into https://github.com/rollup/plugins/blob/f75d5a7fff89bff5f3d59cb268926337b709598a/packages/alias/src/index.ts#L106 and per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#parameters it can be a function, if it goes anywhere else I missed please, do tell

[yyx990803]

Ah, ok, this only happens if vite's own path includes $$. I've been testing with locally linked vite which isn't affected by the project path.

[yyx990803]

Thanks for the fix! The windows CI is failing because of unrelated commits.

[merceyz]

Ah, ok, this only happens if vite's own path includes $$.

That's what I said in the PR description (and why I applied the change to the CLIENT_DIR), but it can happen in any of the .replace calls that don't escape the replacement

Thanks for the fix!

No worries, if you run into issues concerning Yarn PnP please don't hessitate to ping us 👍