Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: displaying function sizes #36

Merged
merged 12 commits into from Mar 17, 2024

Conversation

aleksandrjet
Copy link
Contributor

After discussing #28 I prepared mr showing the function sizes in documentation. @justin-schroeder talked about table with sizes at the end of docs, but I think it would be helpful for users to see the sizes right next to the description. Also I think a github link for each function would be useful.

I made a few variants with different sizes:

Icon size - 24px, font-size - 14px (sm), padding - 8px
lead6_p2

Icon size - 24px, font-size - 14px (sm), padding - 4px
lead6_p1

Icon size - 16px, font-size - 12px (xs), padding - 4px
lead4_p1

Usage of sizes from the file is not ready yet, I'm still waiting for comments on the design

What do you think? Should I choose one of the variants or should I still make a table?

Copy link

vercel bot commented Mar 7, 2024

@aleksandrjet is attempting to deploy a commit to the Formkit Team on Vercel.

A member of the Team first needs to authorize it.

@aleksandrjet aleksandrjet changed the title Func sizes docs: displaying function sizes Mar 7, 2024
Copy link

vercel bot commented Mar 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tempo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 7, 2024 2:25pm

@justin-schroeder
Copy link
Member

Awesome! I like the smaller ones @aleksandrjet — and if we need to tweak them we can. I way prefer the small ones over the table. When you get it to a good place where you are wanting me to merge I can do that. If we need to do any further polishing we can. Great work!

import * as funcs from "../../dist/index.cjs"

const formatBytes = (size) => {
return bytes.format(size, { unitSeparator: " " })
Copy link

Choose a reason for hiding this comment

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

Hey @aleksandrjet. Nice addition to the docs. A minor addition to make it less dependent on third-party libraries:

const bytesFormatter = Intl.NumberFormat("en", {
  notation: "compact",
  style: "unit",
  unit: "byte",
  unitDisplay: "narrow",
})

console.log(bytesFormatter.format(5351)) // "5.4KB"

Feel free to delete the comment if it's off-topic.

P.S. I am not the developer of Tempo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good addition, but I see a small problem with the native format display due to the missing space, which I was not able to add through the config

In my opinion, the option with bytes-iec is read better. Also bytes-iec is used in size-limit which is already present in the dependencies

Intl.NumberFormat
format_native

bytes-iec
format_bytes_iec

Copy link

Choose a reason for hiding this comment

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

In this case you could use .replace() or .formatToParts() to add a space, but that would complicate the code and your solution with bytes-iec looks more readable. Good point. 👍🏻

@aleksandrjet
Copy link
Contributor Author

I added the npm run post:build script to create a docs/assets/func-sizes.json file with function sizes. This file is then used in each top-level component (Parse, Helpers, etc.). I did not find info about deployment, but I think that this script should be added to CI.

Also you need to pay attention to displaying sizes of format and parse functions, because their style is different from the other functions. Perhaps these styles should be changed
example_big_header-min
example_small_header-min

p.s. The yearEnd and yearStart functions were not exported and I added re-export to index.ts

p.p.s. I see that there is no link to Helpers in the table of contents, although this is a large section, perhaps it should be added

@justin-schroeder justin-schroeder merged commit e54e7d2 into formkit:main Mar 17, 2024
1 check failed
@justin-schroeder
Copy link
Member

Awesome! great work @aleksandrjet. Ive merged this and iterated a bit more on the styles.

One thought I had on this all — there is an initial size in brotli and gzip which is included in each of those. if you sum the total of all the functions its way more than the total size of the package because each contains quite a bit of compression information. addDays for example, is 524Bytes on the page, but its incremental size is tiny. I dont know exactly how we could make that more clear on the site though

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.

None yet

3 participants