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

refactor(formatter): improve string normalization #3564

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

Conaclos
Copy link
Member

@Conaclos Conaclos commented Aug 1, 2024

Summary

While I was working on #3558, I noted a room for improving string utilities.

This PR improves the implementation of the string utilities:

  • Reduce an unnecessary lookup (counting the number of characters) when we check the number of quotes inside a string
  • Avoid a string allocation when string contents are normalized
  • Avoid a string allocation when quotes are normalized

The PR has also the aim of improving performance of string normalization by avoiding peekable() and iterating over bytes instead of chars.

The PR also normalizes strings before evaluating if quotes are needed for import attribute keys.
This is a follow-up of #3558.

There is more room for improvements for example to avoid code duplication.
The change are already large in this PR, I prefer to defer them in a follow-up PR.

Test Plan

I improved the test coverage of the normalise_string function.

@github-actions github-actions bot added A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS labels Aug 1, 2024
@Conaclos Conaclos changed the title reffactor(formatter): improve string utils refactor(formatter): improve string utils Aug 1, 2024
@Conaclos Conaclos changed the title refactor(formatter): improve string utils refactor(formatter): improve string normalization Aug 1, 2024
@Conaclos Conaclos force-pushed the conaclos/string-utils-refactor branch from 182e0dd to a98fceb Compare August 1, 2024 13:10
Copy link

codspeed-hq bot commented Aug 1, 2024

CodSpeed Performance Report

Merging #3564 will not alter performance

Comparing conaclos/string-utils-refactor (7b073c9) with main (3229b32)

Summary

✅ 103 untouched benchmarks

⁉️ 1 dropped benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main conaclos/string-utils-refactor Change
⁉️ js_formatter[ios.d_11036528575252089605.d.ts] 778.8 ms N/A N/A

@Conaclos Conaclos force-pushed the conaclos/string-utils-refactor branch from a98fceb to 7b073c9 Compare August 1, 2024 13:49
@Conaclos Conaclos merged commit ed7b91a into main Aug 1, 2024
14 of 15 checks passed
@Conaclos Conaclos deleted the conaclos/string-utils-refactor branch August 1, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter L-CSS Language: CSS L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant