feat: biomejs 2 migration #1874
Conversation
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
|
Thanks for the PR! |
lib/helpers/utils.js
Outdated
| ...statement.matchAll(/\${?([^:\/\\}]+)}?/g), | ||
| ...statement.matchAll(/\${?([^:/\\}]+)}?/g), |
There was a problem hiding this comment.
Are you sure this shouldn't be escaped?
lib/helpers/utils.js
Outdated
| const separatorRegex = /[@#:\/]/; | ||
| const separatorRegex = /[@#:/]/; |
lib/helpers/utils.js
Outdated
| if (l.match(/^[^\s\/]+\/\S+/)) { | ||
| if (l.match(/^[^\s/]+\/\S+/)) { |
lib/helpers/utils.js
Outdated
| versionStr = versionStr.replace(/[\[\]]/g, ""); | ||
| versionStr = versionStr.replace(/[[\]]/g, ""); |
There was a problem hiding this comment.
as per chatgpt first one is correct.
The first one is the correct (and idiomatic) way to strip literal [ and ] characters:
versionStr = versionStr.replace(/[\[\]]/g, "");
Why?
• Inside a character class [...], you need to escape both [ and ] so the engine doesn’t treat them as the class delimiters.
• [\[\]] defines “match either \[ (literal [) or \] (literal ]).”
• The alternate /[[\]]/ is confusing to read (and could be rejected by some engines), since the unescaped [ at the start of the class looks like it’d open a nested class.
Stick with /[\[\]]/g.
|
@malice00 - all of those regex changes you spotted where from biome applying it's auto linting/formatter fixes. |
|
@setchy can we ignore those regex auto-linting rules? We don't have time to test across ecosystems and there is a risk these could create regressions. |
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Yes, i have pushed an update which disables the noUselessEscapeInRegex rule |
|
OK, finally all actions are green! Merging now. 😄 |
| import { | ||
| CARGO_CMD, | ||
| DEBUG_MODE, | ||
| DOTNET_CMD, | ||
| GCC_CMD, | ||
| GO_CMD, | ||
| getJavaCommand, |
There was a problem hiding this comment.
This is an interesting decision to sort ignoring the case.
| @@ -5,7 +5,11 @@ | |||
| "packageManager": "yarn@4.0.0-rc.50", | |||
| "type": "module", | |||
| "license": "MIT", | |||
| "workspaces": ["app", "edge", "scripts"], | |||
| "workspaces": [ | |||
There was a problem hiding this comment.
This is fine, but can we exclude biome from touching the test data?
|
@setchy, wdyt about ignoring test and contrib folders in biome? |
Personally I'd want all files in a repository to apply the same formatter and linter rules |
|
Interesting. ok, let's keep as-is and revisit incase it breaks any tests or cause confusion. |
Biome v2 just launched - https://biomejs.dev/blog/biome-v2/
This PR migrates cdxgen to biome2 using their migration cli.
In addition, I have configured the organize import action to group imports into
node,packages,all other, separated by a new line