fix(windows) Fix autolinking and remove legacy projects #521
Conversation
* Updates the module project with a newer template which will be detected by autolinking (RNW >= 0.63) * Fixes react-native.config to properly identify the windows example project * Removes the legacy C# project, which only confuses autolinking * Updates docs around windows autolinking support A side effect of this update is that RNW 0.60/0.61 are no longer "supported".
|
FYI @tero-paananen |
There was a problem hiding this comment.
Wow - looks pretty great, thank you! Curious for a look from @tero-paananen if they have time
To be sure I understand, we're clipping off support for RNW < 0.62 ?
I have no problem with issuing breaking changes I just like to make helpful / authoritative release notes when they happen so there is no doubt on what broke, who's affected and how to handle it
Cheers!
|
For context, in RNW terms, 0.60 and 0.61 are pretty ancient. RNW 0.62 helped standardize things in a way that makes support/compatibility easier. (I should know, I'm the one that did it. And implemented autolinking in 0.63.) While the actual module code may be compatible (I didn't really have to change that) there have been lots of changes to the build files/tooling. In order to continue supporting 0.60/0.61 I'd have to create separate, parallel, build files, and one group (either <= 0.61 users or >= 0.62 users) would have to change their project linking. Anyway, I went with the "easier to maintain in the future" option. Also, I tested this change by verifying:
Hopefully the drop of RNW 0.60/0.61 support isn't too painful, but honestly, if you're a dev out there targeting RNW, please use a newer version. :) |
|
Without being too brutal about it, past a few releases back I'm of the opinion "if it breaks you, don't upgrade" and then get on with the easier to maintain bit. Maintainer time is I think the scarce resource in the ecosystem so I'm totally fine with semver major, dropping RNW<62 and moving right along. Thanks for the explanation! |
|
That PR is ok for me of course. I will try to get time to test this on Windows tomorrow. |
The module and the example app both builds and works. I tested using Visual Studio 16.11.6. |
|
One side note. I am not sure does this module (master) work on Windows if there is SIM card in use. One our customer says that our app works and another that there is some issues when using SIM card. We do not have Windows device with SIM card. |
|
Great - merged it with the appropriate release note I believe, should auto-launch momentarily. Always happy to merge these high quality PRs :-), thanks! |
|
🎉 This PR is included in version 7.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
This is so awesome! <3 |
| sourceDir: 'example\\windows', | ||
| solutionFile: 'NetInfoExample.sln', | ||
| project: { | ||
| projectFile: 'NetInfoExample\\NetInfoExample.vcxproj', |
There was a problem hiding this comment.
@jonthysell @mikehardy
I am not sure if this has been fixed in newer versions of react native. But the usage of \\ here could be problematic for ios users in other indirect scenarios.
See my other PR where we ran into similar issues, we ended up having to patch for a while.
wonday/react-native-orientation-locker#188
Was this fixed in newer versions of RNW? Could this still be problematic for anyone on RNW63 and RNW64 or whichever versions that are still supported but older?
There was a problem hiding this comment.
I had to patch this immediately it was painful here c9a2966
detected by autolinking (RNW >= 0.63)
A side effect of this update is that RNW 0.60/0.61 are no longer "supported".
Closes #516