Avoid using RegExp special symbols in virtual paths, e.g. $$virtual -> __virtual

[non25]

First of all, I want to say thanks to the yarn team for a great work. It made my js package-management workflow actually enjoyable.

And now to the issue itself.

Using $$virtual as a base for virtual paths breaks people's naive RegExp code in the packages, which work completely fine in node_modules scenario.

Maintainers of such packages are not going to react to PRs fast enough, and it just makes it harder to work on actual issues in forks using portal: for no apparent reason.

Such PRs could actually break something else, which is not obvious right away, because lack of tests, etc.

sveltejs/svelte-loader#149
sveltejs/svelte-loader#155
https://github.com/rixo/svelte-hmr/pull/23

It also requires a dedicated @merceyz to save the day, whether he wants it at the moment or not. 🙂

I've been working on svelte-loader, where this situation occured and this was just getting in the way, spoiling my amazing yarn berry experience.

I wanted to just check if my fork is actually working and get done with the day, but after digging through unrelated code had to ask @merceyz to help me fix it.

I feel like the probability of having $ somewhere around node_modules is too low to justify making $-proof RegExp's everywhere in people's packages.

If it isn't gonna break existing commited .pnp.js'es in user's PnP zero-install repos, maybe it is reasonable to save some frustration for almost no cost?

What do you think?

[arcanis]

Yes and no - in retrospect I perhaps would have chosen a different format, but now, one year later, I'm not certain it's a good idea to change. Who knows what makes assumptions about these paths? 🤔

Another thing is that if it breaks something, then perhaps it's something that should really be fixed in the relevant packages. I know you addressed this point, but I think you forgot the security aspect : while it's not super relevant for every package, anything broken due to regex characters is also likely vulnerable to catastrophic backtracking or other more subtle scenarios.

[Knagis]

Just a note - in the referenced tickets, $ causes issue by being in the replacement value, not the regular expression itself, so it is unlikely the same places are vulnerable to regexp injections.

> "from 'placeholder'".replace(/placeholder/, "a/$$virtual/foo")
< "from 'a/$virtual/foo'"

[arcanis]

I think I'm open to reconsider, I don't want us to lose more time and energy over this (problem happened again in a fairly known project). It's still a bad idea for the ecosystem to do this since it'll break with $$-paths regardless of Yarn, but we have better things to do than to fight for it 😅

Would /home/foo/app/__virtual__/xyz/0/ be acceptable? Can someone find a way to break it?

[7rulnik]

@arcanis @merceyz I mentioned #1835 and tried it with iterm, but default regexp doesn't work and it splits filename by $$.

Seems that there is a way to modify default regexp, but it doesn't feel as good DX

Default regexp looks like that (note that it uses icu format):

\~?/?([[:letter:][:number:]._-]+/+)+[[:letter:][:number:]._-]+/?

https://iterm2.com/smartselection.html

[arcanis]

Will be fixed by #2595