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

feat: switch to smol-toml for toml parsing #2

Merged
merged 3 commits into from Feb 20, 2024
Merged

feat: switch to smol-toml for toml parsing #2

merged 3 commits into from Feb 20, 2024

Conversation

uncenter
Copy link
Contributor

@uncenter uncenter commented Feb 20, 2024

πŸ”— Linked issue

N/A

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

The TOML parser currently in use, https://github.com/BinaryMuse/toml-node, only supports the 0.4.0 spec, and therefore has many open issues regarding unsupported features. On the other hand, https://github.com/squirrelchat/smol-toml supports the latest 1.0.0 spec and has zero spec-related issues open. In addition, I believe the performance is significantly better with smol-toml:

β€’ toml
-------------------------------------------------------------------
confbox                       2'060 ns/iter   (1'729 ns … 3'123 ns)
BinaryMuse/toml-node         18'563 ns/iter    (15'625 ns … 340 Β΅s)
sunnyadn/js-toml             18'722 ns/iter    (17'333 ns … 337 Β΅s)
squirrelchat/smol-toml        1'470 ns/iter   (1'409 ns … 1'731 ns)

summary for toml
  confbox
   1.4x slower than squirrelchat/smol-toml
   9.01x faster than BinaryMuse/toml-node
   9.09x faster than sunnyadn/js-toml

If I'm reading that correctly, the current parser is 1.4 times slower than smol-toml!

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

Copy link

codecov bot commented Feb 20, 2024

Welcome to Codecov πŸŽ‰

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered β˜‚οΈ

Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Didn't know about it thanks!

@pi0 pi0 merged commit 384ee61 into unjs:main Feb 20, 2024
2 checks passed
@uncenter uncenter deleted the feat/smol-toml branch February 20, 2024 19:12
@uncenter
Copy link
Contributor Author

Didn't know about it thanks!

Yeah it isn't very well known at all. Thanks for approving!

@uncenter
Copy link
Contributor Author

Also uh - should we add a stringifyTOML function? Not sure why that only exists for YAML, I'm guessing the other libraries don't support it? smol-toml does I think.

@pi0
Copy link
Member

pi0 commented Feb 20, 2024

Mainly bundle size. And yaml wasn't easily tree-shakable so thought to expose it when it is there πŸ˜† If you have any usecases for toml serialization in the future we can add it of course.

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

2 participants