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

Update examples.md #2411

Closed
wants to merge 1 commit into from
Closed

Update examples.md #2411

wants to merge 1 commit into from

Conversation

enigmatix
Copy link

Include a cautionary tale for anyone attempting to build react native with these libraries.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • [ x] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • [ x] Documentation content changes
  • Other... Please describe:

What is the current behavior?

React Native projects currently don't build with rules_nodejs due to a conflict of using symlinks for source files and the node_modules folder. The only successful way to build a react native project is to have all source files together in one place.
Issue Number: N/A

What is the new behavior?

N/A

Does this PR introduce a breaking change?

  • Yes
  • [ x] No

Other information

Include a cautionary tale for anyone attempting to build react native with these libraries.
@mattem
Copy link
Collaborator

mattem commented Jan 22, 2021

Scanning the thread for that issue, there are some mentions of haste-map being used in the bundler to map the files, which we already patch for Jest tests to allow it to discover symlinks.
I have no experience with React Native, but I'd like to explore perhaps a similar patching route before we say rules_nodejs says it can't support something due to symlinks.

@mattem mattem added the need: investigation Requires some digging to determine if action is needed label Jan 22, 2021
@enigmatix
Copy link
Author

Yes that's correct, it does use haste-map. Most of the file-location errors I have are traced back to that.

@mattem
Copy link
Collaborator

mattem commented Feb 17, 2021

Are you able to apply the same or similar patch as in the examples for Jest to solve the issue?

@ajanuar
Copy link

ajanuar commented Mar 13, 2021

@enigmatix I'd like to know whether patch is possible. I'm considering to use this rules_nodejs for my RN project.

@farcaller
Copy link

Using RN is definitely possible, in principle, but not in the way everyone seems to want. I did some research and I think it's workable, although I have a bit of a mess of shell scripts, bazel and ruby, so I'm trying to recreate it all from a clean state now. Here are my working notes, though, if anyone wants to travel the same road:

  1. Metro keeps dying on EPIPE and I couldn't trace where the EPIPE comes from. For now I substituted it with parcel which is a known good packer for bazel.
  2. You will have to own your mobile app build pipeline. Not sure what that means for android yet, for iOS you'll have to do all the objective-c pipeline setup yourself. the PodToBUILD works, sorta, but it has many rough edges and I'm still struggling to make the recent RN podfile to work.
  3. No idea how to do proper codesplitting without Metro yet.

@farcaller
Copy link

A bit more digging, I feel like I'm going down some weird rabbit hole towards insanity.

js bundle

There are numerous bugs open to bring symlinks to metro. In fact, the issue #1 was open for 5 years now with no traction, so it's very clear to me metro devs do not care about this use case.

One could ditch metro, bundle with whatever. Rollup works. The issues are the resource loading (for which metro has special case code) and bundle splitting (for which RN has a special binary format). tbf, both sound like an easier thing to tackle than adding symlink support to metro.

ios app

RN depends on a scripted cocoapods project. There is PodToBUILD that supposedly can integrate cocoapods ecosystem with bazel but I haven't had any luck with that insofar.

android app

RN gave up on publishing aars to maven repositories, but it's not a huge deal, the aar is part of the npm package and you can easily pull it in with

aar_import(
    name = "react_native",
    aar = "@npm//:node_modules/react-native/android/com/facebook/react/react-native/0.64.0/react-native-0.64.0.aar",
)

and dep on it in your android_library rules. The problem comes from several files that are generated by yarn run react-native run-android, specifically PackageList is a generated file. For what it's worth (I did the android hello world yesterday, so I must not be the best source on this whole mess), I don't see any viable way to generate those files without the RN's tooling. But I don't understand what's happening under the hood of the gradle build to come to a solid conclusion.

takeaways

just use flutter (sadly, the whole idea of my project is dynamic ui that's evaluated in runtime)

It seems more and more to me that RN is so heavily invested into its complex build process that it would be a significantly non-trivial effort to make everything build outside of the RN toolchain. Many of those complexities are warranted by the autolinking, however. Due to autolinking, even the integration with the external apps will require one to invest into the RN build toolchain.

One alternative would be to build/pack as much of your frontend sources with bazel as you can, then offload it to an action that creates a temporary workdir for metro, copies the final bundle into it and lets metro repack it all over as part of the react-native run-whatever call. I'm not sure I'm curious enough to go that route.

@alexeagle
Copy link
Collaborator

thanks for your analysis @farcaller I agree that if metro cannot handle symlinks and the ecosystem has made it impossible to build RN apps without it, then it's going to be an awfully big project to make a decomposition of this build system such that you can use Bazel to orchestrate it.

Making a non-symlinked sandbox to run metro in is possible, but will be quite slow so it's hard to get benefits.

@farcaller
Copy link

I think I made little progress on the latter. I'm currently experimenting outside bazel (just with a bunch of symlinked sources, heh), and it looks like metro just ignores symlinks when it stumbles upon them. Which means it will call to the external resolver to try and locate the source (and we can hack that by actually traversing the symlink and returning the real path).

Can't quite confirm it all will work under bazel as it still EPIPEs somewhere inside and I can't figure how to debug that, but it's an open possibility.

Otherwise, it seems that you can theoretically make bazel build the android host the same way Buck does it – by first calling into gradle to assemble all the aar deps in a known location. Although that feature seems to be broken right now.

So maybe, just maybe, it's not all moot and it is actually possible to make it all work.

@farcaller
Copy link

Good news everyone!

image

I'm not quite sure why and how, but I got metro bundler to work (and it was easier than I anticipated). I wonder what broke along the path.

You can check out some very messy implementation in here. I'll need a long nap before I get to cleaning it, but the main point is that metro builds its own sources and it can link in the output of ts_project rule too.

@farcaller farcaller mentioned this pull request Mar 18, 2021
12 tasks
@farcaller
Copy link

I created #2541 to relocate this discussion if this PR gets closed (and I think I'm in position to say it can be feasibly closed given more work).

@mattem
Copy link
Collaborator

mattem commented Apr 2, 2021

Closing to continue discussion in #2541 Awesome investigation and work @farcaller 🚀

@mattem mattem closed this Apr 2, 2021
@enigmatix enigmatix deleted the patch-1 branch April 8, 2021 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes need: investigation Requires some digging to determine if action is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants