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

fix(js_formater): strip useless quotes #3492

Merged
merged 8 commits into from
Jul 23, 2024

Conversation

suxin2017
Copy link
Contributor

@suxin2017 suxin2017 commented Jul 22, 2024

Summary

fix(js_formater): strip useless quotes
#3484

- import J from "file.json" with { "type": "json" }
+ import J from "file.json" with { type: "json" }

Test Plan

@github-actions github-actions bot added A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages labels Jul 22, 2024
Copy link

codspeed-hq bot commented Jul 22, 2024

CodSpeed Performance Report

Merging #3492 will improve performances by 21.2%

Comparing suxin2017:fix_strip_quotes (a20a69c) with suxin2017:fix_strip_quotes (70c7c05)

Summary

⚡ 1 improvements
✅ 103 untouched benchmarks

Benchmarks breakdown

Benchmark suxin2017:fix_strip_quotes suxin2017:fix_strip_quotes Change
eucjp_4223654609205405644.json[cached] 986.7 µs 814.1 µs +21.2%

@Conaclos
Copy link
Member

Thanks! Could you rebase on main? I've just updated the Prettier snapshots.

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project A-LSP Area: language server protocol A-Changelog Area: changelog labels Jul 23, 2024
@github-actions github-actions bot removed A-Project Area: project A-LSP Area: language server protocol A-Changelog Area: changelog labels Jul 23, 2024
@suxin2017
Copy link
Contributor Author

I have modified it, but I did not modify the code related to paser, I wonder why the performance has decreased

@ematipico
Copy link
Member

ematipico commented Jul 23, 2024

I have modified it, but I did not modify the code related to paser, I wonder why the performance has decreased

It's a flaky benchmark , it's unrelated to your changes

Copy link
Member

@Conaclos Conaclos left a comment

Choose a reason for hiding this comment

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

Looks good to me. I've just suggested a change of name because import assertion is the old name of import attribute.

crates/biome_js_formatter/src/utils/string_utils.rs Outdated Show resolved Hide resolved
crates/biome_js_formatter/src/utils/string_utils.rs Outdated Show resolved Hide resolved
crates/biome_js_formatter/src/utils/string_utils.rs Outdated Show resolved Hide resolved
@suxin2017
Copy link
Contributor Author

Looks good to me. I've just suggested a change of name because import assertion is the old name of import attribute.

Thank you for your review, I've done.

@Conaclos
Copy link
Member

Everything is green, except rust code formatting. Once formatted, we can merge the PR.

@Conaclos Conaclos merged commit a743b49 into biomejs:main Jul 23, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants