Skip to content

formatted files and wrote some docs#1

Open
publicJorn wants to merge 3 commits intomainfrom
formatting
Open

formatted files and wrote some docs#1
publicJorn wants to merge 3 commits intomainfrom
formatting

Conversation

@publicJorn
Copy link
Member

@publicJorn publicJorn commented Sep 17, 2025

I didn't really change anything.

I checked out the project because I was looking for the reason why sorting didn't work as expected. Played around with it whilst bun-linked to Onbezorgd Ontslag.

So ran build which created some formatting changes, but nothing functionally has changed.

Anyway, whilst playing around it appeared that biome behaved weird. I think maybe you (@maanlamp) have a different version installed globally?

I updated biome and fixed all lint thingies.

Also added some docs to make it a bit more clear how it works.

Will follow up with a PR, because I think default sort should be "Asc" and anyway, the default direction should be settable per column.

@publicJorn publicJorn requested a review from maanlamp September 17, 2025 09:17
@publicJorn
Copy link
Member Author

publicJorn commented Sep 17, 2025

I take it you push this to npm manually. Can you do that after merge? thanks!

edit
Don't think a push is necessary, because nothing functionally changed. Any bundle size changes because of formatting will be negligable.

state: o,
children: s
}) => /* @__PURE__ */ c(
}, i = ({ state: o, children: s }) => /* @__PURE__ */ c(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is biome acting up again, but there shouldn't be any formatting changes in generated files from dist/*

In this case you need to figure out the sort direction yourself.
### Render columns

It is advisable to define a columns variable with `useMemo()` so that it will
Copy link
Contributor

Choose a reason for hiding this comment

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

While of course this is good etiquette, I'm not sure I agree that we should push for memoization.

In my experience, it's almost never worth it in terms of performance, and often causes confusion for our colleagues when they don't quite understand the purpose and effects of it.

In the projects that uses this table, we don't really use memoization because of these points... Maybe we should do a "react hooks deep-dive" tech-talk to cover this? I could prepare one if you'd like.

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