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: preserve i18n.json formatting to avoid unnecessary PRs #280

Closed

Conversation

Aditya-A-G
Copy link

fixes: #197

This PR updates the CLI's JSON serializer to detect and respect existing indentation in i18n.json, avoiding unnecessary formatting changes, addressing the core need outlined in #197. Other formatting elements like quote style aren’t needed for this solution.

@Aditya-A-G
Copy link
Author

Hey @maxprilutskiy, could you review this PR when you have a moment? Thanks!

@maxprilutskiy
Copy link
Contributor

maxprilutskiy commented Nov 1, 2024

hey @Aditya-A-G – thanks for the PR!

Actually, given some recent changes we made, you can now implement #197 in a much more elegant / easier way.

  1. We already have prettier among dependecies of this project
  2. Prettier has a JS API
  3. Upon (re-)saving the i18n.json configuration, let's use the prettier config the user might already have, and if they don't then let's use the default prettier config. (Our only requirement to the the default fallback is 2 spaces indentation).

Once you finish the implementation, don't forget to run pnpm new from the root of the repo, and then add a brief summary of the changes you're making. Your message will then trigger a new npm package release, once the PR is merged, and will become a part of the changelog.

Let me know if this makes sense!

@Aditya-A-G
Copy link
Author

Yes, that makes sense! I will implement the change using Prettier as you suggested.

@maxprilutskiy
Copy link
Contributor

@Aditya-A-G let me know how this goes and if you need any help!

cc @partik03 if you have any opinion here, lmk

@partik03
Copy link
Contributor

partik03 commented Nov 4, 2024

@maxprilutskiy I tried searching for this using prettier but prettier.resolveConfig function is an asynchronous function which needs to be awaited to resolve the configuration of the i18n file due to which we will have to handle promises at various instances in the code

edit: check the prettier doc here

@Aditya-A-G
Copy link
Author

will create a fresh pr

@Aditya-A-G Aditya-A-G closed this Nov 4, 2024
@Aditya-A-G Aditya-A-G deleted the fix/preserve-i18n-formatting branch November 4, 2024 14:29
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.

3 participants