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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Replace unicode-id crate with unicode-id-start #78

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

Boshen
Copy link
Contributor

@Boshen Boshen commented Feb 27, 2024

We've gone full circle 馃槄

The unicode-id and unicode-id-start crates were cloned from unicode-xid and unicode-ident for usages in the oxc project.

Oxc will be using this crate for sourcemap support, and is currently using the better optimized unicode-id-start crate. I don't want to include two similar dependencies due to the rather large static storage size of these crates.

The unicode-id-start crate is also faster and uses slightly less static storage. See full details at https://github.com/oxc-project/unicode-id-start/tree/master?tab=readme-ov-file#comparison-of-performance


The version "1" is used instead of the latest version because the crate is stable, future versions will only include unicode version upgrades.


And thank you Sentry for the sponsorship due to the inclusion of unicode-id crate 鉂わ笍

We've gone full circle 馃槄

The `unicode-id` and `unicode-id-start` crates were cloned from (`unicode-xid`)(https://crates.io/crates/unicode-xid) and [`unicode-ident`](https://crates.io/crates/unicode-ident) for usages in the [oxc project](https://github.com/oxc-project/oxc).

Oxc will be using this crate for sourcemap support, and is currently using the better optimized `unicode-id-start` crate. I don't want to include two similar dependencies due to the rather large static storage size of these crates.

The `unicode-id-start` crate is also faster and uses slightly less static storage. See full details at https://github.com/oxc-project/unicode-id-start/tree/master?tab=readme-ov-file#comparison-of-performance

---

The version "1" is used instead of the latest version because the crate is stable, future versions will only include unicode version upgrades.
@Boshen
Copy link
Contributor Author

Boshen commented Feb 27, 2024

For detecting JavaScript identifiers, oxc has an optimized routine which this crate could use. I can add the code to unicode-id-start if we want more performance :-) https://github.com/oxc-project/oxc/blob/27052ebfed27d8aa97913d26f95d285d9cb58e51/crates/oxc_syntax/src/identifier.rs#L66-L137

src/js_identifiers.rs Outdated Show resolved Hide resolved
@Swatinem
Copy link
Member

lgtm

Though I believe this whole module, and sourceview which is the only other module that uses this might be headed out the door once we get around to #71

@Boshen
Copy link
Contributor Author

Boshen commented Feb 27, 2024

Fixed the clippy warning, ready to merge, and possibly a release 馃榿

@Swatinem Swatinem merged commit eff3dc0 into getsentry:master Feb 27, 2024
4 checks passed
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