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

style: Automated formatting and linting [PR #4 - Linting and Formatting] #66

Merged
merged 33 commits into from
Jan 25, 2024

Conversation

tibuurcio
Copy link
Collaborator

Summary

Testing Plan

  • Was this tested locally? If not, explain why.

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

@tibuurcio tibuurcio self-assigned this Jan 18, 2024
@tibuurcio tibuurcio changed the title style: Automated formatting and linting style: Automated formatting and linting (PR #4) Jan 18, 2024
@tibuurcio tibuurcio changed the title style: Automated formatting and linting (PR #4) style: Automated formatting and linting [PR #4 - Linting and Formatting] Jan 18, 2024
@jared-dickman
Copy link
Collaborator

On <></> usage

This will probably be my most controversial one 😅
...

i switched from preferring () to <></> for a couple small reasons.

the fragment is actually real jsx syntax, and you can add some special attributes to it, like a key, which can be useful instead of needing a "full div".

also there are cases where you want to add a sibling element next to the component. the fragment allows this but the parens dont, because react elements can only support a single root node [which can be a fragment]

to the point of it not being valid js syntax, i dont buy that one yet. for comparison, we dont do this

function  addOne(n: number): number {
  return (
    n + 1
  )
}

so why should we in jsx files??

i guess if prettier really really really thinks () are better than <></>, fine. we can try it.
in which case i would want to remove all the current fragments...

@tibuurcio tibuurcio marked this pull request as ready for review January 19, 2024 15:33
@jared-dickman
Copy link
Collaborator

this looks gtg once the conflicts are resolved

@tibuurcio
Copy link
Collaborator Author

this looks gtg once the conflicts are resolved

I will wait on #68 in order to fix everything just cause there should be some changes there as well

@tibuurcio tibuurcio merged commit 4cf6c21 into main Jan 25, 2024
9 checks passed
Copy link

github-actions bot commented Feb 6, 2024

🎉 This PR is included in version 1.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants