build(client,build-tools): Upgrade biome to 1.9.3 #22721
Conversation
github-actions
bot
added
area: build
github-actions
bot
added
the
dependencies
| describe(includeStringification | ||
| ? "with stringification" | ||
| : "without stringification", () => { | ||
| for (const [name, data, context] of encodingTestData.successes) { |
There was a problem hiding this comment.
Hmm I'm torn about this one. The ones before where the function() declaration ended up at the end of a line didn't feel so bad for some reason, then I hit this and it felt weird, and now the other ones also feel a bit weird 🤔 . Is it configurable by any chance?
There was a problem hiding this comment.
The only config we can do to influence this (maybe) is to apply a different line length to this file, but that would have other side effects. If we really don't like the changes in this file, we could use inline ignores I think.
There was a problem hiding this comment.
I don't think I'd bother if it's just me. If someone else has similar opinion maybe we can think about it.
There was a problem hiding this comment.
The new line wrapping here seems fine, though I also preferred the old one I don't think its worth worrying about.
I do find it odd that the line 896 has the same indent as the one before it despite being inside a new level of nesting from the lambda. To me that looks like a biome bug which should probably be reported.
My suggestion is to go ahead with merging this update (assuming everything else is fine) but report this case as a bug.
There was a problem hiding this comment.
Also the choice to line wrap one member of an argument list, and put part of the next member on the same line as the last part of the first one seems odd. I'd expect putting each argument on its own line if following an argument that was line wrapped. which is how the old version worked.
There was a problem hiding this comment.
After finding a lot for cases of this same issue, but worse, I'm no longer sure we want to land this update as is. If it does bug fixes we need, then maybe, this it makes a lot of test bodies really hard to seperate from the line wrapped descriptions
packages/tools/devtools/devtools-view/src/components/data-visualization/EditableView.tsx
Outdated
Show resolved
Hide resolved
…alization/EditableView.tsx Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com>
⯅ @fluid-example/bundle-size-tests: +245 Bytes
Baseline commit: ad35a7c |
| }, | ||
| ); | ||
| it("Should wait for client 1 to leave before moving to connected state(Client 3) when client 2 " + | ||
| "got disconnected from connected state", async () => { |
There was a problem hiding this comment.
In my opinion, this is much harder to read, since the body of the function has the same indent as the line before it and the function declaration is at the end of a line after unrelated content. This really blurs the function body with the test arguments in a hard to visually parse way.
There was a problem hiding this comment.
Seems like the same formatting bug we hit with the tree test, just worse since the first thing in the lambda isn't a loop.
| }); | ||
|
|
||
| it("Client 3 should wait for client 2(which got disconnected without sending any ops) to leave " + | ||
| "when client 2 already waited on client 1", async () => { |
There was a problem hiding this comment.
same issue again here
| } | ||
| it(`No progress for ${maxReconnects} connection state changes, with pending state, should ` + | ||
| "generate telemetry event and throw an error that closes the container", async () => { | ||
| const pendingStateManager = getMockPendingStateManager(); |
There was a problem hiding this comment.
again, a line starting with contents of a function at the same indent as a line before which is part of something else.
Upgrade biome to 1.9.3.
Notable changes: