Skip to content

chore: Migrate to oxfmt#247

Open
lanesawyer wants to merge 1 commit intomainfrom
lane/oxfmt
Open

chore: Migrate to oxfmt#247
lanesawyer wants to merge 1 commit intomainfrom
lane/oxfmt

Conversation

@lanesawyer
Copy link
Copy Markdown
Collaborator

What

Replaces biome formatter with oxfmt

How

Pulled the oxfmt file from our other projects
Installed oxfmt
Removed biome formatting stuff

PR Checklist

  • Is your PR title following our conventional commit naming recommendations?
  • Have you filled in the PR Description Template?
  • Is your branch up to date with the latest in main?
  • Do the CI checks pass successfully?
  • Have you smoke tested the example applications?
  • Did you check that the changes meet accessibility standards?
  • Have you tested the application on these browsers?
    • Chrome (Fully supported)
    • Firefox (Major bug fixes supported)
    • Safari (Major bug fixes supported)

@lanesawyer lanesawyer requested a review from a team as a code owner April 29, 2026 16:53
@lanesawyer lanesawyer requested review from chrisj and froyo-np April 29, 2026 16:53
@lanesawyer lanesawyer enabled auto-merge (squash) April 29, 2026 16:55
Copy link
Copy Markdown
Contributor

@Jarbuckle Jarbuckle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks fine, but have a few notes below. I find it interesting how the default formatting for HTML/JSX is to expand basically everything that has any number of attributes/props into multiple lines; I feel like that might be a negative from a readability perspective, but it's not that big an issue for me.

Comment on lines +60 to +64
export class AsyncDataCache<SemanticKey extends RecordKey, CacheKey extends RecordKey, D> implements AsyncCache<
SemanticKey,
CacheKey,
D
> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one doesn't really feel like a readability improvement to me 🤔

Comment thread packages/dzi/changelog.md
- Add helpful linting rules ([#127](https://github.com/AllenInstitute/vis/pull/127))
- Dev command, reorganized docs, added stubs ([#163](https://github.com/AllenInstitute/vis/pull/163))
- *(deps)* Bump @biomejs/biome from 1.9.4 to 2.0.6 ([#174](https://github.com/AllenInstitute/vis/pull/174))
- _(deps)_ Bump @biomejs/biome from 1.9.4 to 2.0.6 ([#174](https://github.com/AllenInstitute/vis/pull/174))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay, this feels like a good argument in favor of formatting .md files -- using underscores instead of asterisks for italicization is much less than confusing, since it's not competing with the asterisks for bold.

Comment on lines +23 to +42
<Card
title="ABC Atlas"
icon="rocket"
>
The [Allen Brain Cell Atlas (ABC Atlas)](https://knowledge.brain-map.org/abcatlas) uses
`@alleninstitute/vis-core` to visualize millions of points of data in the browser.
</Card>
<Card
title="GTA"
icon="magnifier"
>
The [Genetic Tools Atlas
(GTA)](https://knowledge.brain-map.org/data/7CVKSF7QGAKIQ8LM5LC/specimens/HTS03VQD4BBB67I3BAP) uses
`@alleninstitute/vis-omezarr` to display OME-Zarr previews.
</Card>
<Card
title="DZI"
icon="information"
>
The [SEA-AD project](https://knowledge.brain-map.org/data/JGN327NUXRZSHEV88TN/specimens/JGCXWER774NLNWX2NNR)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be very keen on multi-lining any elements that have multiple props 🤔

Comment thread package.json
Comment on lines +14 to +15
"check": "oxfmt --check && biome lint .",
"check:fix": "oxfmt && biome lint . --write",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, cool idea!

Comment thread package.json
},
"volta": {
"node": "22.11.0",
"node": "24.15.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node upgrade too, eh?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to go to at least 22.13 (I think), figured might as well hop on the 24 LTS train now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants