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: add support for native .node modules to remix-dev (making tailwind integration pain free) #703

Closed
wants to merge 2 commits into from

Conversation

sync
Copy link

@sync sync commented Nov 27, 2021

Was checking out remix-tailwind and it had issues on OSX due to fsevents use of .node modules in OSX. I think remix-tailwind is a super nice way to do tailwind in remix, pain free and it would be nice if our OSX users could also use it.

Here you can find more details about the issue:
itsMapleLeaf/remix-tailwind#1

This code was originated from here and written by Evan:
evanw/esbuild#1051

For anyone interested trying this fix, feel free to:

  • install patch-package
  • create a patches folder in the root
  • unzip and drop the patched files into the patches folder

@remix-run+dev+1.0.6.patch.zip

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Nov 27, 2021

Hi @sync,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run.

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Nov 27, 2021

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@sync
Copy link
Author

sync commented Nov 27, 2021

Also would love to know how you would want to test this ? I tried to run the test locally but few are already failing

@sync sync force-pushed the feature/fsevents-fix-esbuild branch from 5cbbe5e to aabbb22 Compare November 28, 2021 07:12
@sync sync force-pushed the feature/fsevents-fix-esbuild branch from aabbb22 to e5db18e Compare December 1, 2021 11:09
@kentcdodds kentcdodds force-pushed the dev branch 2 times, most recently from ab9dac4 to 172ecc9 Compare December 2, 2021 06:15
@cevr
Copy link

cevr commented Dec 3, 2021

I would get rid of all the doc changes you made as they seem largely unrelated and would make it harder to get your merged (might be considered rude to change coding conventions like that)

@sync sync force-pushed the feature/fsevents-fix-esbuild branch from e5db18e to 6310919 Compare December 4, 2021 00:58
@sync
Copy link
Author

sync commented Dec 4, 2021

I would get rid of all the doc changes you made as they seem largely unrelated and would make it harder to get your merged (might be considered rude to change coding conventions like that)

Yes thanks for the heads up, dev got force pushed a couple of times, messed up my fork

Screen Shot 2021-12-04 at 11 58 46 am

@itsMapleLeaf
Copy link
Contributor

If you haven't, try to git rebase the other commits out of this branch?

@sync
Copy link
Author

sync commented Dec 4, 2021

If you haven't, try to git rebase the other commits out of this branch?

yes all good did it already :-)

@ryanflorence
Copy link
Member

Thanks for taking the time here and sorry for not looking at this sooner (also sorry about the dev branch force pushes--that shouldn't have ever happened).

Since we deploy to non-node environments too, I'm not sure what the implications are here. I'm closing this in favor of you opening up a discussion so we can figure out if we can do this or not given our multi-environment nature.

You mind opening the discussion over here? https://github.com/remix-run/remix/discussions/new?category=ideas

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.

4 participants