feat(windows): Add Windows as a supported platform #103
Conversation
|
@sbeca Thanks so much for this. I was looking at the discussion on the ReactXP repo and was going to suggest a PR like this. I'll take a proper look once I'm done travelling tomorrow. I'll also see why the tests are failing because that seems to be unrelated. On that note, if you know a good way to test React Native apps on Windows then let me know. It doesn't have to be in this PR, but it would be good to know the options we have. Thanks again! |
There was a problem hiding this comment.
Aside from one comment about there ethernet type, this looks good.
You are going to need to add the windows directory to the files array in package.json (here to ensure that the NPM package includes the windows directory.
Don't worry about the failing tests. I think it was just CircleCI having a "moment".
I don't actually have a Windows environment to test on. If someone could confirm that this is working for them, then I'll be happy to release 🎉
| { | ||
| // If we have a connection that is not a Wifi connection or a Cellular | ||
| // connection, then let's fallback and assume it's an Ethernet connection | ||
| return NetworkConnectionType.Ethernet; |
There was a problem hiding this comment.
We are using the other type on Android for when we have an active connection but we don't know what type. Maybe ethernet does make more sense on Windows, but if we could detect that then it would be better.
There was a problem hiding this comment.
I've done some more research and come across a more exact way of detecting an Ethernet connection. However, my laptop doesn't have an Ethernet port so I can't fully test it
|
Thanks for the review @matt-oakes, I've just pushed a commit to address the comments. As Matt can't test this PR, would @mikehardy be able to do so? |
|
I have a win10 VM laying around. The network connection on it is a virtual ethernet thing, I can probably give something a whirl. That said, I have done zero react-native-windows work, I'm not even sure where to begin. My base assumption would be "install react-native-windows", then "clone reactxp, carefully pointing it to the branch with the changes / making sure netinfo is pointing to this netinfo branch", right? Not sure if I'll need build tools there or anything, and it may take a day or three, I'm traveling Mon/Tue |
|
Getting set up to be able to build React Native apps for Windows can take a little while. I can work on some instructions if need be. A questions for @matt-oakes though, how much external validation do you want before accepting this PR? I understand that I'm a new contributor to this repo so just accepting anything I write might give you pause. However, if you did merge it as is then I would be happy to be pinged for help with any issues that may arise regarding Windows support. |
|
@sbeca I'd actually be pretty happy to merge this in as is. The only blocker is the failing tests, but I don't know if that's actually anything to do with this code. I'll take a look at that. Aside from that, this code should not affect any current user at all so I can't see an issue with merging it. I'll keep you in the loop if any bugs come up 👍 |
|
The tests seems to have passed this time (CircleCI must have been having a moment...). I'm going to merge this now and release it as a minor version bump. If anyone has any issues then please open a new issue 👍 Thanks again to @sbeca for implementing this 🎉 |
|
Thank you very much for merging this Matt |
# [3.2.0](v3.1.3...v3.2.0) (2019-05-26) ### Features * Add Windows support ([#103](#103) by [@sbeca](https://github.com/sbeca)) ([cf0fb8f](cf0fb8f))
|
🎉 This PR is included in version 3.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Overview
This PR adds Windows as a supported platform.
I started by bringing over the code from the react-native-windows repo and then I implemented the new features that have been added to this repo since it was extracted from the core of React Native.
Test Plan
I tested this PR manually by adding it to the ReactXP Sample Project and combining it with a WIP PR I currently have open at microsoft/reactxp#1092
I have not added any automated tests with this PR because I have struggled to get this repo's automated tests to run on Windows.