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.

Sign up for GitHub

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

Jump to bottom

feature: order respects side effect imports #320

Merged
merged 1 commit into from
Dec 2, 2024
Merged

feature: order respects side effect imports #320

merged 1 commit into from
Dec 2, 2024

Conversation

vladislavarsenev
Copy link
Contributor

@vladislavarsenev vladislavarsenev commented Nov 27, 2024 •
edited
Loading

What have done

  • cherry-picked changes from Do not reorder side effect nodes #110 #111 Thanks to @blutorange for provided feature.
  • by default sorting respects importOrderPreventSortingSideEffectNodes and turned off by default
  • formatted all files by prettier that weren't formatted yet

@vladislavarsenev vladislavarsenev changed the title feature: order respects side effects feature: order respects side effect importa Nov 27, 2024
@vladislavarsenev vladislavarsenev changed the title feature: order respects side effect importa feature: order respects side effect imports Nov 27, 2024
@byara byara requested review from ayusharma and byara November 28, 2024 08:19
ayusharma
ayusharma approved these changes Nov 29, 2024
View reviewed changes
Copy link
Collaborator

@ayusharma ayusharma left a comment

Choose a reason for hiding this comment

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

Can you please also update the docs in Readme ? ❤️ Overall looks good to me.

byara
byara approved these changes Nov 29, 2024
View reviewed changes
Copy link
Collaborator

@byara byara 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 👍 Just a suggestion to rename the importOrderPreventSortingSideEffectNodes

src/index.ts Outdated Show resolved Hide resolved
@vladislavarsenev
Copy link
Contributor Author

Can you please also update the docs in Readme ? ❤️ Overall looks good to me.

Added information about property

@byara byara merged commit 8072158 into trivago:v5 Dec 2, 2024
3 checks passed
@byara byara mentioned this pull request Dec 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@byara byara byara approved these changes

@ayusharma ayusharma ayusharma approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vladislavarsenev @ayusharma @byara