fix: parsing mixed-value toml arrays

[JP-Ellis]

Upgrade @iarna/toml to v3 which should support mixed values within TOML arrays.

Updated one of the test fixtures to make use of the newer [dependency-groups] section.

Fixes: #25413

[JP-Ellis]

@microsoft-github-policy-service agree

[JP-Ellis]

@eleanorjboyd Any chance you could take a look at this?

[Tyriar]

@eleanorjboyd we should not accept dependency changes from external contributors like in VS Code.

[JP-Ellis]

That's on odd policy... Is that to mitigate supply chain attacks? I must've missed it in your contribution guide.

What about when a PR actually fixes an issue? How do I get it accepted?

[eleanorjboyd]

Hi! Sorry for the confusion, I can look at handling this on my end. One question, when I run run npm view @iarna/toml version (singular), it shows the version tagged as latest in the npm registry, which is 2.2.5. However, when I run npm view @iarna/toml time (which shows all versions), I can see that 3.0.0 exists but was published after 2.2.5. Any idea is 3.0.0 is stable then if it isn't tagged as latest?

[JP-Ellis]

I am not the developer behind this dependency, so I can only speculate:

1. Based on the fact the 2.2.4, 2.2.5 and 3.0.0 were all releases in quick succession, all after about ~1 year of inactivity, I suspect the developer came back to the project after a hiatus and fixed a bunch of issues quickly.
2. 3.0.0 has been released and available for over 5 years now, so I suspect it is stable enough (and if you're worried about supply chain attacks... well that's a lot of planning 😆).

I have no idea how npm determines which version is the latest. It could be that the developer at the time forgot to mark 3.0.0 as the latest version?

[Tyriar]

@JP-Ellis yes it's to mitigate supply chain attacks. This is something we do on microsoft/vscode to prevent one possible avenue for attack, and we should do it here as well. No offence intended 😉

Until we have an action that blocks it, this could be handled by @eleanorjboyd checking out the upgrade (seems safe if it's been years) and then running the npm install on her side to generate the lock file on a trusted machine.

[eleanorjboyd]

Thanks @Tyriar for the full explanation! I am going to make the changes on my end and share the PR.

[eleanorjboyd]

opened this PR which will fix this: #25584

[JP-Ellis]

My PR is not just a package version upgrade. I'm also updating the test to validate that the issue is actually fixed.

[eleanorjboyd]

very true! Would you prefer I port over this change to my new branch or do you want to change this PR to just include the test for validation? Want to make sure you get credit!

[JP-Ellis]

I'm happy for you to build on top of this PR, and run the necessary dependency verification and lockfile updates.

[eleanorjboyd]

ok cherry picked it to here: #25585

[JP-Ellis]

Awesome thanks!