feat: use string replaces instead of splits

[43081j]

Using a split/join results in a lot of garbage collection for the unused arrays, along with being fairly slow. This just moves to using a RegExp replace instead.

Bench result:

3,410 ops/sec

Bench result on main:

1,150 ops/sec

let me know if you want any changes. i'm not so happy about the amount of new consts lying around, but figured it is simpler than having some set of pairs i iterate through.

also @juliangruber, would you ever consider doing patches to 2.x? many millions of the downloads are still CJS (2.x), it would be good to get this perf gain out to them since they're unlikely to move to ESM sooner

[juliangruber]

Just a quick heads up: The strategy LGTM, but I haven't had time to think about the risk of this PR. Could this introduce bugs that the current test suite isn't covering?

[43081j]

it shouldn't really, we're basically using split/join as a way to emulate replaceAll i think

i haven't used replaceAll since it isn't available in older versions of node, so the equivalent is to just have some RegExps with the global modifier.

i'm not sure how we can definitely prove this though 🤔 'a-b-c'.split('-').join('_') is the same as 'a-b-c'.replaceAll('-', '_')

even with no match, ''.split('-') is [''], and [''].join('_') is ''. which is still the same as a replaceAll

[43081j]

@juliangruber just chasing up on this, any chance we can move it forward?

also would be good to know if there's a way we can provide this perf change to 2.x too. though no worries if its too much of a pain to backport

[juliangruber]

Could you share with me why the speed up is important to you?

[43081j]

As part of an ecosystem cleanup effort, we're moving a bunch of projects from braces to brace-expansion.

In some of those projects, enough expansions happen that this slowdown becomes easy to observe. So the maintainers generally didn't want to use brace-expansion because of that.

For example, i'm in the middle of helping rewrite chokidar and wanted to use brace-expansion, but it slowed the benchmark by a large amount (although that is irrelevant now admittedly since we don't want to support globs anymore).

[juliangruber]

Thank you for sharing! That is very valid indeed.

I'm thinking about releasing this as a new major version, to just bring the risk down as much as possible. The the existing users shouldn't mind if they stay on a slower version if they don't upgrade.

Would that work for you?

[43081j]

that's fair enough, i think it sounds like a sensible strategy 👍

i'll be pushing for migrating a few repos to this too so will make sure i get them on the latest major at the same time

[juliangruber]

Thank you! https://github.com/juliangruber/brace-expansion/releases/tag/v4.0.0