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

chore(atomic): fix build on windows #3128

Merged
merged 2 commits into from
Aug 25, 2023
Merged

chore(atomic): fix build on windows #3128

merged 2 commits into from
Aug 25, 2023

Conversation

louis-bompart
Copy link
Collaborator

@louis-bompart louis-bompart commented Aug 23, 2023

Empirical fix, I didn't trace back exactly where it goes haywire.
Gut feeling: too much parallelism made using fs.promises, which in turn leads to Windows to :rage4: because lower limits for opened files.

Solution: Reuse the synchronous code on Windows, and let Linux be Linux.
I prefer to not invest more, will prolly be fixed when Oxide (Tailwind not-so-secret Rust sauce that should come 🔜 ™️) will land.

https://coveord.atlassian.net/browse/KIT-2691

@louis-bompart louis-bompart requested a review from a team as a code owner August 23, 2023 21:29
@github-actions
Copy link

github-actions bot commented Aug 23, 2023

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 181.8 181.8 0
search 344.5 344.5 0
insight 297.9 297.9 0
product-listing 286.9 286.9 0
product-recommendation 156.1 156.1 0
recommendation 192.5 192.5 0
ssr 184 184 0

@louis-bompart louis-bompart changed the title chore(atomic): revert tailwindlabs/tailwindcss#11548 chore(atomic): fix build on windows Aug 23, 2023
@louis-bompart
Copy link
Collaborator Author

@lvu285 can you validate that all's well on your side?

Copy link
Contributor

@mrrajamanickam-coveo mrrajamanickam-coveo left a comment

Choose a reason for hiding this comment

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

Looks like potentially useful fix for upstream as well!

Copy link
Contributor

@dmbrooke dmbrooke left a comment

Choose a reason for hiding this comment

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

Can't test on windows myself, but I think this looks good! LGTM 🎉

@louis-bompart
Copy link
Collaborator Author

Looks like potentially useful fix for upstream as well!

Agreed, I had planned to raised up the issue and discuss it with tailwind gang today.

@lvu285
Copy link
Contributor

lvu285 commented Aug 24, 2023

@louis-bompart confirmed it fix the issue on my Windows. Thanks!

@louis-bompart louis-bompart enabled auto-merge (squash) August 24, 2023 19:28
@louis-bompart louis-bompart merged commit 6772723 into master Aug 25, 2023
@louis-bompart louis-bompart deleted the KIT-2691 branch August 25, 2023 17:27
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.

5 participants